[RFC PATCH v2 0/6] i2c: Enabling use of aspeed-i2c with ACPI

[RFC PATCH v2 0/6] i2c: Enabling use of aspeed-i2c with ACPI

[Jonathan Cameron]

v2:
 - Various updates called out in individual patches based on review
   from Andy Shevchenko.  Still hacks, but nicer ones now. 

From the school of dirty hacks we do to prove something works, enable
work to proceed elsewhere:

MCTP over I2C from ACPI emulated hosts (both x86 and ARM64).

The first 4 patches might be suitable for upstream inclusion, the
last two not - though I hope we can move to Niyas' work on ACPI clock
management once that is ready.

Why do this crazy thing?

Ultimately we want a standards based way to use the CXL Fabric Management
API FM-API. In real systems that is likely to be driven from a separate
'host' such as a BMC, but for test purposes it is convenient to be able
to do that from an QEMU emulated machine that is also capable of using
the CXL kernel stack.

That CXL kernel stack is currently ACPI only (and people care about
x86 for some reason). One of the defined interfaces over which FM-API
commands can be issued is MCTP.

The kernel MCTP stack has upstream drivers for MCTP over I2C.
Upstream QEMU emulates the Aspeed I2C controller with the necessary
two way support. There are patches on list adding the MCTP parts
https://lore.kernel.org/qemu-devel/20230425063540.46143-2-its@irrelevant.dk/
and I've ported an earlier CXL FMAPI EP emulator over to that.

ACPI has a 'magic' HID of PRP0001 which allows the use of a device tree binding
(mostly) with an ACPI DSDT entry.  A suitable chunk is something like

(dumped from a working x86 setup)

    Device (MCTP)
    {
        Name (_HID, "PRP0001")  // _HID: Hardware ID
        Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
        {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
            Package (0x03)
            {
                Package (0x02)
                {
                     "compatible",
                     "aspeed,ast2600-i2c-bus"
                },
                Package (0x02)
                {
                    "bus-frequency",
                    0x00061A80
                },
                Package (0x02)
                {
                    "mctp-controller",
                    One
                }
            }
        })
        Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
        {
            QWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite,
                0x0000000000000000, // Granularity
                0x00000004800FC080, // Range Minimum
                0x00000004800FC0FF, // Range Maximum
                0x0000000000000000, // Translation Offset
                0x0000000000000080, // Length
                ,, , AddressRangeMemory, TypeStatic)
            Interrupt (ResourceConsumer, Level, ActiveHigh, Shared, ,, )
            {
                0x00000007,
            }
        })
    }
    Device (MCTS)
    {
        Name (_HID, "PRP0001")  // _HID: Hardware ID
        Name (_CRS, ResourceTemplate ()  // _CRS: Current Resource Settings
        {
            I2cSerialBusV2 (0x0050, DeviceInitiated, 0x000186A0,
                AddressingMode7Bit, "\\_SB.MCTP",
                0x00, ResourceProducer, , Exclusive,
                )
        })
        Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
        {
            ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */,
            Package (0x01)
            {
                Package (0x02)
                {
                    "compatible",
                    "mctp-i2c-controller"
                }
            }
        })
    }

QEMU patches will follow soon and will include documentation on
how to actually poke this to do something useful. I'll post a reply
to this with the link when posted.

Cc: Niyas Sait <niyas.sait@linaro.org>
Cc: Klaus Jensen <its@irrelevant.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Jeremy Kerr <jk@codeconstruct.com.au>
Cc: Matt Johnston <matt@codeconstruct.com.au>
Cc: Shesha Bhushan Sreenivasamurthy <sheshas@marvell.com>

Jonathan Cameron (6):
  i2c: acpi: set slave mode flag
  i2c: aspeed: Use platform_get_irq() instead of opencoding
  i2c: aspeed: Don't report error when optional dt bus-frequency not
    supplied
  i2c: aspeed: switch to generic fw properties.
  i2c: aspeed: Set the fwnode for the adap->dev
  HACK: i2c: aspeed: Comment clock out and make reset optional

 drivers/i2c/busses/i2c-aspeed.c | 43 +++++++++++++++------------------
 drivers/i2c/i2c-core-acpi.c     |  3 +++
 2 files changed, 22 insertions(+), 24 deletions(-)

-- 
2.39.2

[RFC PATCH v2 1/6] i2c: acpi: set slave mode flag

[Jonathan Cameron]

If the GenericSerialBusConnection includes the General Flag
for slave mode set it during parsing.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v2: Picked up tag.
---
 drivers/i2c/i2c-core-acpi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
index d6037a328669..7dda5eab9645 100644
--- a/drivers/i2c/i2c-core-acpi.c
+++ b/drivers/i2c/i2c-core-acpi.c
@@ -125,6 +125,9 @@ static int i2c_acpi_fill_info(struct acpi_resource *ares, void *data)
 	if (sb->access_mode == ACPI_I2C_10BIT_MODE)
 		info->flags |= I2C_CLIENT_TEN;
 
+	if (sb->slave_mode)
+		info->flags |= I2C_CLIENT_SLAVE;
+
 	return 1;
 }
 
-- 
2.39.2

[RFC PATCH v2 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding

[Jonathan Cameron]

A cleanup in it's own right.
This has the handy side effect of working for ACPI FW as well
(unlike fwnode_irq_get() which works for ARM64 but not x86 ACPI)

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v2: Andy Schevchenko review.
Added check on whether we have an IRQ from platform_get_irq()
---
 drivers/i2c/busses/i2c-aspeed.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index d3c99c5b3247..21a2f139f445 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -19,7 +19,6 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
-#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
@@ -1043,7 +1042,10 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
 	ret = devm_request_irq(&pdev->dev, irq, aspeed_i2c_bus_irq,
 			       0, dev_name(&pdev->dev), bus);
 	if (ret < 0)
-- 
2.39.2

[RFC PATCH v2 3/6] i2c: aspeed: Don't report error when optional dt bus-frequency not supplied

[Jonathan Cameron]

The bindings have this property as optional with a default of 100kHz.
As such the driver should not be printing an error message if it is not
supplied.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v2: New patch after Andy raised some questions about this property
    and I read the binding doc.
---
 drivers/i2c/busses/i2c-aspeed.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 21a2f139f445..4363bfe06f9b 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -1003,13 +1003,9 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	}
 	reset_control_deassert(bus->rst);
 
-	ret = of_property_read_u32(pdev->dev.of_node,
-				   "bus-frequency", &bus->bus_frequency);
-	if (ret < 0) {
-		dev_err(&pdev->dev,
-			"Could not read bus-frequency property\n");
-		bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
-	}
+	bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
+	of_property_read_u32(pdev->dev.of_node,
+			     "bus-frequency", &bus->bus_frequency);
 
 	match = of_match_node(aspeed_i2c_bus_of_table, pdev->dev.of_node);
 	if (!match)
-- 
2.39.2

[RFC PATCH v2 4/6] i2c: aspeed: switch to generic fw properties.

[Jonathan Cameron]

Moving over to generic firmware properties allows this driver to
get closer to working out of the box with both device tree and
other firmware options, such as ACPI via PRP0001.

Tested only via QEMU emulation.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v2:
- Added a proper patch description.
- Updated to reflect optional nature of bus-frequency.
---
 drivers/i2c/busses/i2c-aspeed.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 4363bfe06f9b..353834d7ffd5 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -18,9 +18,8 @@
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/of_address.h>
-#include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
 
@@ -975,7 +974,6 @@ MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
 
 static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 {
-	const struct of_device_id *match;
 	struct aspeed_i2c_bus *bus;
 	struct clk *parent_clk;
 	int irq, ret;
@@ -1004,15 +1002,13 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	reset_control_deassert(bus->rst);
 
 	bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
-	of_property_read_u32(pdev->dev.of_node,
-			     "bus-frequency", &bus->bus_frequency);
+	device_property_read_u32(&pdev->dev,
+				 "bus-frequency", &bus->bus_frequency);
 
-	match = of_match_node(aspeed_i2c_bus_of_table, pdev->dev.of_node);
-	if (!match)
+	bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
+		device_get_match_data(&pdev->dev);
+	if (!bus->get_clk_reg_val)
 		bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
-	else
-		bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
-				match->data;
 
 	/* Initialize the I2C adapter */
 	spin_lock_init(&bus->lock);
-- 
2.39.2

[RFC PATCH v2 5/6] i2c: aspeed: Set the fwnode for the adap->dev

[Jonathan Cameron]

This is needed for the bus matching used for ACPI based
i2c client registration.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v2: Drop the separate setting of the of_node. (Thanks Andy)
---
 drivers/i2c/busses/i2c-aspeed.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 353834d7ffd5..ae8e187f2233 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -1017,7 +1017,6 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	bus->adap.retries = 0;
 	bus->adap.algo = &aspeed_i2c_algo;
 	bus->adap.dev.parent = &pdev->dev;
-	bus->adap.dev.of_node = pdev->dev.of_node;
 	strscpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
 	i2c_set_adapdata(&bus->adap, bus);
 
@@ -1043,6 +1042,8 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	device_set_node(&bus->adap.dev, dev_fwnode(&pdev->dev));
+
 	ret = i2c_add_adapter(&bus->adap);
 	if (ret < 0)
 		return ret;
-- 
2.39.2

[RFC PATCH v2 6/6] HACK: i2c: aspeed: Comment clock out and make reset optional

[Jonathan Cameron]

Needs tidying up - hopefully can do clock right using ongoing
work from Niyas
https://linaro.atlassian.net/wiki/spaces/CLIENTPC/pages/28832333867/ACPI+Clock+Management

ACPI does not provide an equivalent reset deassert / assert. _RST
doesn't fit that model, so for now make the reset optional.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v2:
 - Use optional variant for the reset.
 - Left the clock with the hideous hack to keep it obvious that it is
   a hack given no way for us to get the clk rate on ACPI firmware yet
   and I don't want to pretend there is.
---
 drivers/i2c/busses/i2c-aspeed.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index ae8e187f2233..f0b5789aee21 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -986,14 +986,14 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	if (IS_ERR(bus->base))
 		return PTR_ERR(bus->base);
 
-	parent_clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(parent_clk))
-		return PTR_ERR(parent_clk);
-	bus->parent_clk_frequency = clk_get_rate(parent_clk);
+	//	parent_clk = devm_clk_get(&pdev->dev, NULL);
+	//	if (IS_ERR(parent_clk))//
+	//		return PTR_ERR(parent_clk);
+	bus->parent_clk_frequency = 1000000;//clk_get_rate(parent_clk);
 	/* We just need the clock rate, we don't actually use the clk object. */
-	devm_clk_put(&pdev->dev, parent_clk);
+	//devm_clk_put(&pdev->dev, parent_clk);
 
-	bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
+	bus->rst = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
 	if (IS_ERR(bus->rst)) {
 		dev_err(&pdev->dev,
 			"missing or invalid reset controller device tree entry\n");
-- 
2.39.2

Re: [RFC PATCH v2 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding

[Andy Shevchenko]

On Tue, May 30, 2023 at 5:57 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> A cleanup in it's own right.

its ?

> This has the handy side effect of working for ACPI FW as well
> (unlike fwnode_irq_get() which works for ARM64 but not x86 ACPI)

...

>  #include <linux/of_address.h>

>  #include <linux/of_platform.h>

With your commit message I'm wondering why these are still there. Do
we have some OF calls that are optional and do not affect
functionality (in ACPI case)?


-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH v2 3/6] i2c: aspeed: Don't report error when optional dt bus-frequency not supplied

[Andy Shevchenko]

On Tue, May 30, 2023 at 5:57 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> The bindings have this property as optional with a default of 100kHz.
> As such the driver should not be printing an error message if it is not
> supplied.

Right
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v2: New patch after Andy raised some questions about this property
>     and I read the binding doc.
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 21a2f139f445..4363bfe06f9b 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -1003,13 +1003,9 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>         }
>         reset_control_deassert(bus->rst);
>
> -       ret = of_property_read_u32(pdev->dev.of_node,
> -                                  "bus-frequency", &bus->bus_frequency);
> -       if (ret < 0) {
> -               dev_err(&pdev->dev,
> -                       "Could not read bus-frequency property\n");
> -               bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> -       }
> +       bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
> +       of_property_read_u32(pdev->dev.of_node,
> +                            "bus-frequency", &bus->bus_frequency);
>
>         match = of_match_node(aspeed_i2c_bus_of_table, pdev->dev.of_node);
>         if (!match)
> --
> 2.39.2
>


-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH v2 4/6] i2c: aspeed: switch to generic fw properties.

[Andy Shevchenko]

On Tue, May 30, 2023 at 5:58 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> Moving over to generic firmware properties allows this driver to
> get closer to working out of the box with both device tree and
> other firmware options, such as ACPI via PRP0001.
>
> Tested only via QEMU emulation.

...

> -#include <linux/of_address.h>
> -#include <linux/of_platform.h>

> +#include <linux/property.h>

Ah, stupid me. You can discard the previous question (in one of the
previous emails).

...

> +       bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
> +               device_get_match_data(&pdev->dev);

Why not typedef this function and use it here and there?

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH v2 5/6] i2c: aspeed: Set the fwnode for the adap->dev

[Andy Shevchenko]

On Tue, May 30, 2023 at 5:58 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> This is needed for the bus matching used for ACPI based
> i2c client registration.

The right thing to do.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v2: Drop the separate setting of the of_node. (Thanks Andy)
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 353834d7ffd5..ae8e187f2233 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -1017,7 +1017,6 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>         bus->adap.retries = 0;
>         bus->adap.algo = &aspeed_i2c_algo;
>         bus->adap.dev.parent = &pdev->dev;
> -       bus->adap.dev.of_node = pdev->dev.of_node;
>         strscpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
>         i2c_set_adapdata(&bus->adap, bus);
>
> @@ -1043,6 +1042,8 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>         if (ret < 0)
>                 return ret;
>
> +       device_set_node(&bus->adap.dev, dev_fwnode(&pdev->dev));
> +
>         ret = i2c_add_adapter(&bus->adap);
>         if (ret < 0)
>                 return ret;
> --
> 2.39.2
>


-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH v2 6/6] HACK: i2c: aspeed: Comment clock out and make reset optional

[Andy Shevchenko]

On Tue, May 30, 2023 at 5:59 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> Needs tidying up - hopefully can do clock right using ongoing
> work from Niyas
> https://linaro.atlassian.net/wiki/spaces/CLIENTPC/pages/28832333867/ACPI+Clock+Management

I'm wondering how this will be solved for the cases where the
"clock-frequency" property is used, see below.

> ACPI does not provide an equivalent reset deassert / assert. _RST
> doesn't fit that model, so for now make the reset optional.

...

>  - Left the clock with the hideous hack to keep it obvious that it is
>    a hack given no way for us to get the clk rate on ACPI firmware yet
>    and I don't want to pretend there is.

The workaround in some cases is to read the "clock-frequency"
property, which is standard de facto in several drivers / subsystems.

That said, why not split this patch into two and switch the clock to
be optional and use the above property? Okay, one thing is that this
can collide probably with the generic I2C bus timings, which this
driver uses with a non-standard property name.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH v2 6/6] HACK: i2c: aspeed: Comment clock out and make reset optional

[Jonathan Cameron]

On Tue, 30 May 2023 22:59:50 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, May 30, 2023 at 5:59 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > Needs tidying up - hopefully can do clock right using ongoing
> > work from Niyas
> > https://linaro.atlassian.net/wiki/spaces/CLIENTPC/pages/28832333867/ACPI+Clock+Management  
> 
> I'm wondering how this will be solved for the cases where the
> "clock-frequency" property is used, see below.
> 
> > ACPI does not provide an equivalent reset deassert / assert. _RST
> > doesn't fit that model, so for now make the reset optional.  
> 
> ...
> 
> >  - Left the clock with the hideous hack to keep it obvious that it is
> >    a hack given no way for us to get the clk rate on ACPI firmware yet
> >    and I don't want to pretend there is.  
> 
> The workaround in some cases is to read the "clock-frequency"
> property, which is standard de facto in several drivers / subsystems.

That's the wrong clock frequency.  I believe that property is the i2c bus clock
frequency Documentation/devicetree/bindings/i2c/i2c.txt 
The one being used here is the input clock.  I'd imagine there is some division
done, but probably doesn't make sense to represent that using the clock framework
as the i2c bus clock signal isn't directly derived from the input clock
(pulse stretching and all sorts of other fun in I2C).  Also massive overkill
given no one else can use this clock.

> 
> That said, why not split this patch into two and switch the clock to
> be optional and use the above property? Okay, one thing is that this
> can collide probably with the generic I2C bus timings, which this
> driver uses with a non-standard property name.
> 

I'd rather provide the clock from ACPI using Niyas' stuff when ready -
it looks like a promising general solution so don't want to put an
partial solution in before that.

I kept these two changes in the last patch as I suspect they are the
ones that can be considered a hack, given we don't actually have a
real platform using ACPI with this device.  Anyone on aspeed know if
anyone cares about ACPI on those BMCs?

Jonathan

Re: [RFC PATCH v2 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding

[Jonathan Cameron]

On Tue, 30 May 2023 22:45:30 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, May 30, 2023 at 5:57 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > A cleanup in it's own right.  
> 
> its ?

Pah. English - a language that I can't write in despite it being
my native tongue ;)

> 
> > This has the handy side effect of working for ACPI FW as well
> > (unlike fwnode_irq_get() which works for ARM64 but not x86 ACPI)  
> 
> ...
> 
> >  #include <linux/of_address.h>  
> 
> >  #include <linux/of_platform.h>  
> 
> With your commit message I'm wondering why these are still there. Do
> we have some OF calls that are optional and do not affect
> functionality (in ACPI case)?
> 
> 

Re: [RFC PATCH v2 4/6] i2c: aspeed: switch to generic fw properties.

[Jonathan Cameron]

On Tue, 30 May 2023 22:50:12 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, May 30, 2023 at 5:58 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > Moving over to generic firmware properties allows this driver to
> > get closer to working out of the box with both device tree and
> > other firmware options, such as ACPI via PRP0001.
> >
> > Tested only via QEMU emulation.  
> 
> ...
> 
> > -#include <linux/of_address.h>
> > -#include <linux/of_platform.h>  
> 
> > +#include <linux/property.h>  
> 
> Ah, stupid me. You can discard the previous question (in one of the
> previous emails).
> 
> ...
> 
> > +       bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
> > +               device_get_match_data(&pdev->dev);  
> 
> Why not typedef this function and use it here and there?
> 
Minimizing scope of changes, but sure it's a reasonable tidy up so
a precursor patch.


Jonathan