All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI
@ 2023-05-25 15:21 Jonathan Cameron
  2023-05-25 15:21 ` [RFC PATCH 1/6] i2c: acpi: set slave mode flag Jonathan Cameron
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-25 15:21 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

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: switch to generic fw properties.
  i2c: aspeed: Set the fwnode for the adap->dev
  HACK: i2c: aspeed: Comment the clock and reset out.
  HACK: i2c: aspeed: Enable build without COMPILE_TEST

 drivers/i2c/busses/Kconfig      |  1 -
 drivers/i2c/busses/i2c-aspeed.c | 36 ++++++++++++++++-----------------
 drivers/i2c/i2c-core-acpi.c     |  3 +++
 3 files changed, 20 insertions(+), 20 deletions(-)


base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
-- 
2.39.2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 1/6] i2c: acpi: set slave mode flag
  2023-05-25 15:21 [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
@ 2023-05-25 15:21 ` Jonathan Cameron
  2023-05-26 19:43   ` Andy Shevchenko
  2023-05-25 15:21 ` [RFC PATCH 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding Jonathan Cameron
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-25 15:21 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

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

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding
  2023-05-25 15:21 [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
  2023-05-25 15:21 ` [RFC PATCH 1/6] i2c: acpi: set slave mode flag Jonathan Cameron
@ 2023-05-25 15:21 ` Jonathan Cameron
  2023-05-26 21:06   ` Andy Shevchenko
  2023-05-25 15:22 ` [RFC PATCH 3/6] i2c: aspeed: switch to generic fw properties Jonathan Cameron
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-25 15:21 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

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>
---
 drivers/i2c/busses/i2c-aspeed.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index d3c99c5b3247..04155c9a50a5 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,7 @@ 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);
 	ret = devm_request_irq(&pdev->dev, irq, aspeed_i2c_bus_irq,
 			       0, dev_name(&pdev->dev), bus);
 	if (ret < 0)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 3/6] i2c: aspeed: switch to generic fw properties.
  2023-05-25 15:21 [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
  2023-05-25 15:21 ` [RFC PATCH 1/6] i2c: acpi: set slave mode flag Jonathan Cameron
  2023-05-25 15:21 ` [RFC PATCH 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding Jonathan Cameron
@ 2023-05-25 15:22 ` Jonathan Cameron
  2023-05-26 21:11   ` Andy Shevchenko
  2023-05-25 15:22 ` [RFC PATCH 4/6] i2c: aspeed: Set the fwnode for the adap->dev Jonathan Cameron
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-25 15:22 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

Not tested on device tree but works nicely for ACPI :)

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 04155c9a50a5..73508fb9dc08 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;
@@ -1003,7 +1001,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	}
 	reset_control_deassert(bus->rst);
 
-	ret = of_property_read_u32(pdev->dev.of_node,
+	ret = device_property_read_u32(&pdev->dev,
 				   "bus-frequency", &bus->bus_frequency);
 	if (ret < 0) {
 		dev_err(&pdev->dev,
@@ -1011,12 +1009,10 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 		bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
 	}
 
-	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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 4/6] i2c: aspeed: Set the fwnode for the adap->dev
  2023-05-25 15:21 [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
                   ` (2 preceding siblings ...)
  2023-05-25 15:22 ` [RFC PATCH 3/6] i2c: aspeed: switch to generic fw properties Jonathan Cameron
@ 2023-05-25 15:22 ` Jonathan Cameron
  2023-05-26 21:13   ` Andy Shevchenko
  2023-05-25 15:22 ` [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out Jonathan Cameron
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-25 15:22 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

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

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 73508fb9dc08..adbb93444d7a 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -1044,6 +1044,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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out.
  2023-05-25 15:21 [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
                   ` (3 preceding siblings ...)
  2023-05-25 15:22 ` [RFC PATCH 4/6] i2c: aspeed: Set the fwnode for the adap->dev Jonathan Cameron
@ 2023-05-25 15:22 ` Jonathan Cameron
  2023-05-26 21:16   ` Andy Shevchenko
  2023-05-26 23:36   ` kernel test robot
  2023-05-25 15:22 ` [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST Jonathan Cameron
  2023-05-25 16:18 ` [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
  6 siblings, 2 replies; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-25 15:22 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

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

Don't think ACPI provide an equivalent reset deasert / assert. _RST
doesn't fit that model.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/i2c/busses/i2c-aspeed.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index adbb93444d7a..df20382970f3 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -986,20 +986,21 @@ 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);
 	if (IS_ERR(bus->rst)) {
-		dev_err(&pdev->dev,
+		dev_warn(&pdev->dev,
 			"missing or invalid reset controller device tree entry\n");
-		return PTR_ERR(bus->rst);
+		bus->rst = 0;
+	} else {
+		reset_control_deassert(bus->rst);
 	}
-	reset_control_deassert(bus->rst);
 
 	ret = device_property_read_u32(&pdev->dev,
 				   "bus-frequency", &bus->bus_frequency);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST
  2023-05-25 15:21 [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
                   ` (4 preceding siblings ...)
  2023-05-25 15:22 ` [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out Jonathan Cameron
@ 2023-05-25 15:22 ` Jonathan Cameron
  2023-05-26 10:37   ` kernel test robot
                     ` (2 more replies)
  2023-05-25 16:18 ` [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
  6 siblings, 3 replies; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-25 15:22 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

See cover letter for why...
This works nicely on x86 with appropriate evil emulation.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/i2c/busses/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 87600b4aacb3..96bb5a05e195 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -391,7 +391,6 @@ config I2C_ALTERA
 
 config I2C_ASPEED
 	tristate "Aspeed I2C Controller"
-	depends on ARCH_ASPEED || COMPILE_TEST
 	help
 	  If you say yes to this option, support will be included for the
 	  Aspeed I2C controller.
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI
  2023-05-25 15:21 [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
                   ` (5 preceding siblings ...)
  2023-05-25 15:22 ` [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST Jonathan Cameron
@ 2023-05-25 16:18 ` Jonathan Cameron
  6 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-25 16:18 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl,
	Viacheslav A . Dubeyko

On Thu, 25 May 2023 16:21:57 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> 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.

https://lore.kernel.org/linux-cxl/20230525160859.32517-1-Jonathan.Cameron@huawei.com/T/#t
https://gitlab.com/jic23/qemu cxl-2023-05-25

> 
> 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: switch to generic fw properties.
>   i2c: aspeed: Set the fwnode for the adap->dev
>   HACK: i2c: aspeed: Comment the clock and reset out.
>   HACK: i2c: aspeed: Enable build without COMPILE_TEST
> 
>  drivers/i2c/busses/Kconfig      |  1 -
>  drivers/i2c/busses/i2c-aspeed.c | 36 ++++++++++++++++-----------------
>  drivers/i2c/i2c-core-acpi.c     |  3 +++
>  3 files changed, 20 insertions(+), 20 deletions(-)
> 
> 
> base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST
  2023-05-25 15:22 ` [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST Jonathan Cameron
@ 2023-05-26 10:37   ` kernel test robot
  2023-05-26 21:30   ` kernel test robot
  2023-05-30 14:44   ` Jonathan Cameron
  2 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2023-05-26 10:37 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: oe-kbuild-all

Hi Jonathan,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6]

url:    https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cameron/i2c-acpi-set-slave-mode-flag/20230525-233511
base:   f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
patch link:    https://lore.kernel.org/r/20230525152203.32190-7-Jonathan.Cameron%40huawei.com
patch subject: [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230526/202305261835.nLcaXbN6-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/c866c2b49e19eaa618df6661c42b24b7506beba3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jonathan-Cameron/i2c-acpi-set-slave-mode-flag/20230525-233511
        git checkout c866c2b49e19eaa618df6661c42b24b7506beba3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/i2c/busses/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305261835.nLcaXbN6-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/i2c/busses/i2c-aspeed.c: In function 'aspeed_i2c_probe_bus':
>> drivers/i2c/busses/i2c-aspeed.c:978:21: warning: unused variable 'parent_clk' [-Wunused-variable]
     978 |         struct clk *parent_clk;
         |                     ^~~~~~~~~~


vim +/parent_clk +978 drivers/i2c/busses/i2c-aspeed.c

87b59ff8d1d9d8 Brendan Higgins  2017-07-28   974  
f327c686d3ba44 Brendan Higgins  2017-06-20   975  static int aspeed_i2c_probe_bus(struct platform_device *pdev)
f327c686d3ba44 Brendan Higgins  2017-06-20   976  {
f327c686d3ba44 Brendan Higgins  2017-06-20   977  	struct aspeed_i2c_bus *bus;
f327c686d3ba44 Brendan Higgins  2017-06-20  @978  	struct clk *parent_clk;
f327c686d3ba44 Brendan Higgins  2017-06-20   979  	int irq, ret;
f327c686d3ba44 Brendan Higgins  2017-06-20   980  
f327c686d3ba44 Brendan Higgins  2017-06-20   981  	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
f327c686d3ba44 Brendan Higgins  2017-06-20   982  	if (!bus)
f327c686d3ba44 Brendan Higgins  2017-06-20   983  		return -ENOMEM;
f327c686d3ba44 Brendan Higgins  2017-06-20   984  
6b1e1925d82976 ye xingchen      2023-01-19   985  	bus->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
f327c686d3ba44 Brendan Higgins  2017-06-20   986  	if (IS_ERR(bus->base))
f327c686d3ba44 Brendan Higgins  2017-06-20   987  		return PTR_ERR(bus->base);
f327c686d3ba44 Brendan Higgins  2017-06-20   988  
d400cf85e735f5 Jonathan Cameron 2023-05-25   989  	//	parent_clk = devm_clk_get(&pdev->dev, NULL);
d400cf85e735f5 Jonathan Cameron 2023-05-25   990  	//	if (IS_ERR(parent_clk))//
d400cf85e735f5 Jonathan Cameron 2023-05-25   991  	//		return PTR_ERR(parent_clk);
d400cf85e735f5 Jonathan Cameron 2023-05-25   992  	bus->parent_clk_frequency = 1000000;//clk_get_rate(parent_clk);
f327c686d3ba44 Brendan Higgins  2017-06-20   993  	/* We just need the clock rate, we don't actually use the clk object. */
d400cf85e735f5 Jonathan Cameron 2023-05-25   994  	//devm_clk_put(&pdev->dev, parent_clk);
f327c686d3ba44 Brendan Higgins  2017-06-20   995  
edd20e95bca4a5 Joel Stanley     2017-11-01   996  	bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
edd20e95bca4a5 Joel Stanley     2017-11-01   997  	if (IS_ERR(bus->rst)) {
d400cf85e735f5 Jonathan Cameron 2023-05-25   998  		dev_warn(&pdev->dev,
6bc33c51976cd9 Jae Hyun Yoo     2018-07-02   999  			"missing or invalid reset controller device tree entry\n");
d400cf85e735f5 Jonathan Cameron 2023-05-25  1000  		bus->rst = 0;
d400cf85e735f5 Jonathan Cameron 2023-05-25  1001  	} else {
edd20e95bca4a5 Joel Stanley     2017-11-01  1002  		reset_control_deassert(bus->rst);
d400cf85e735f5 Jonathan Cameron 2023-05-25  1003  	}
edd20e95bca4a5 Joel Stanley     2017-11-01  1004  
248c1a4a98731d Jonathan Cameron 2023-05-25  1005  	ret = device_property_read_u32(&pdev->dev,
f327c686d3ba44 Brendan Higgins  2017-06-20  1006  				   "bus-frequency", &bus->bus_frequency);
f327c686d3ba44 Brendan Higgins  2017-06-20  1007  	if (ret < 0) {
f327c686d3ba44 Brendan Higgins  2017-06-20  1008  		dev_err(&pdev->dev,
f327c686d3ba44 Brendan Higgins  2017-06-20  1009  			"Could not read bus-frequency property\n");
90224e6468e15d Andy Shevchenko  2020-03-24  1010  		bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
f327c686d3ba44 Brendan Higgins  2017-06-20  1011  	}
f327c686d3ba44 Brendan Higgins  2017-06-20  1012  
17ccba67109cd0 Brendan Higgins  2018-09-21  1013  	bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
248c1a4a98731d Jonathan Cameron 2023-05-25  1014  		device_get_match_data(&pdev->dev);
248c1a4a98731d Jonathan Cameron 2023-05-25  1015  	if (!bus->get_clk_reg_val)
248c1a4a98731d Jonathan Cameron 2023-05-25  1016  		bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
87b59ff8d1d9d8 Brendan Higgins  2017-07-28  1017  
f327c686d3ba44 Brendan Higgins  2017-06-20  1018  	/* Initialize the I2C adapter */
f327c686d3ba44 Brendan Higgins  2017-06-20  1019  	spin_lock_init(&bus->lock);
f327c686d3ba44 Brendan Higgins  2017-06-20  1020  	init_completion(&bus->cmd_complete);
f327c686d3ba44 Brendan Higgins  2017-06-20  1021  	bus->adap.owner = THIS_MODULE;
f327c686d3ba44 Brendan Higgins  2017-06-20  1022  	bus->adap.retries = 0;
f327c686d3ba44 Brendan Higgins  2017-06-20  1023  	bus->adap.algo = &aspeed_i2c_algo;
f327c686d3ba44 Brendan Higgins  2017-06-20  1024  	bus->adap.dev.parent = &pdev->dev;
f327c686d3ba44 Brendan Higgins  2017-06-20  1025  	bus->adap.dev.of_node = pdev->dev.of_node;
ea1558ce149d28 Wolfram Sang     2022-08-11  1026  	strscpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
f327c686d3ba44 Brendan Higgins  2017-06-20  1027  	i2c_set_adapdata(&bus->adap, bus);
f327c686d3ba44 Brendan Higgins  2017-06-20  1028  
f327c686d3ba44 Brendan Higgins  2017-06-20  1029  	bus->dev = &pdev->dev;
f327c686d3ba44 Brendan Higgins  2017-06-20  1030  
f327c686d3ba44 Brendan Higgins  2017-06-20  1031  	/* Clean up any left over interrupt state. */
f327c686d3ba44 Brendan Higgins  2017-06-20  1032  	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
f327c686d3ba44 Brendan Higgins  2017-06-20  1033  	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
f327c686d3ba44 Brendan Higgins  2017-06-20  1034  	/*
f327c686d3ba44 Brendan Higgins  2017-06-20  1035  	 * bus.lock does not need to be held because the interrupt handler has
f327c686d3ba44 Brendan Higgins  2017-06-20  1036  	 * not been enabled yet.
f327c686d3ba44 Brendan Higgins  2017-06-20  1037  	 */
f327c686d3ba44 Brendan Higgins  2017-06-20  1038  	ret = aspeed_i2c_init(bus, pdev);
f327c686d3ba44 Brendan Higgins  2017-06-20  1039  	if (ret < 0)
f327c686d3ba44 Brendan Higgins  2017-06-20  1040  		return ret;
f327c686d3ba44 Brendan Higgins  2017-06-20  1041  
f2016870518449 Jonathan Cameron 2023-05-25  1042  	irq = platform_get_irq(pdev, 0);
f327c686d3ba44 Brendan Higgins  2017-06-20  1043  	ret = devm_request_irq(&pdev->dev, irq, aspeed_i2c_bus_irq,
f327c686d3ba44 Brendan Higgins  2017-06-20  1044  			       0, dev_name(&pdev->dev), bus);
f327c686d3ba44 Brendan Higgins  2017-06-20  1045  	if (ret < 0)
f327c686d3ba44 Brendan Higgins  2017-06-20  1046  		return ret;
f327c686d3ba44 Brendan Higgins  2017-06-20  1047  
348e97f7dd4ae4 Jonathan Cameron 2023-05-25  1048  	device_set_node(&bus->adap.dev, dev_fwnode(&pdev->dev));
348e97f7dd4ae4 Jonathan Cameron 2023-05-25  1049  
f327c686d3ba44 Brendan Higgins  2017-06-20  1050  	ret = i2c_add_adapter(&bus->adap);
f327c686d3ba44 Brendan Higgins  2017-06-20  1051  	if (ret < 0)
f327c686d3ba44 Brendan Higgins  2017-06-20  1052  		return ret;
f327c686d3ba44 Brendan Higgins  2017-06-20  1053  
f327c686d3ba44 Brendan Higgins  2017-06-20  1054  	platform_set_drvdata(pdev, bus);
f327c686d3ba44 Brendan Higgins  2017-06-20  1055  
f327c686d3ba44 Brendan Higgins  2017-06-20  1056  	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
f327c686d3ba44 Brendan Higgins  2017-06-20  1057  		 bus->adap.nr, irq);
f327c686d3ba44 Brendan Higgins  2017-06-20  1058  
f327c686d3ba44 Brendan Higgins  2017-06-20  1059  	return 0;
f327c686d3ba44 Brendan Higgins  2017-06-20  1060  }
f327c686d3ba44 Brendan Higgins  2017-06-20  1061  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 1/6] i2c: acpi: set slave mode flag
  2023-05-25 15:21 ` [RFC PATCH 1/6] i2c: acpi: set slave mode flag Jonathan Cameron
@ 2023-05-26 19:43   ` Andy Shevchenko
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2023-05-26 19:43 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-i2c, Wolfram Sang, Niyas Sait, Klaus Jensen,
	Andy Shevchenko, linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

On Thu, May 25, 2023 at 6:22 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> If the GenericSerialBusConnection includes the General Flag
> for slave mode set it during parsing.

Since it's obvious that you are using the existing ACPI specification
bits, it's a nice patch!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Nevertheless the remark is that most likely this feature was never
tested on the ACPI implementations other than ACPICA (evidently by
you). Would it be interesting to know Microsoft's point of view on
this.

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  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
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding
  2023-05-25 15:21 ` [RFC PATCH 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding Jonathan Cameron
@ 2023-05-26 21:06   ` Andy Shevchenko
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2023-05-26 21:06 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-i2c, Wolfram Sang, Niyas Sait, Klaus Jensen,
	Andy Shevchenko, linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

On Thu, May 25, 2023 at 6:23 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)

...

> -       irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> +       irq = platform_get_irq(pdev, 0);

It makes sense now not to even try a wrong IRQ, i.e.

  if (irq < 0)
    return irq;

+ blank line here.

>         ret = devm_request_irq(&pdev->dev, irq, aspeed_i2c_bus_irq,
>                                0, dev_name(&pdev->dev), bus);
>         if (ret < 0)


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 3/6] i2c: aspeed: switch to generic fw properties.
  2023-05-25 15:22 ` [RFC PATCH 3/6] i2c: aspeed: switch to generic fw properties Jonathan Cameron
@ 2023-05-26 21:11   ` Andy Shevchenko
  2023-05-30 14:16     ` Jonathan Cameron
  0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2023-05-26 21:11 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-i2c, Wolfram Sang, Niyas Sait, Klaus Jensen,
	Andy Shevchenko, linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

On Thu, May 25, 2023 at 6:23 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> Not tested on device tree but works nicely for ACPI :)

Needs a better commit message obviously :-)

...

> -       ret = of_property_read_u32(pdev->dev.of_node,
> +       ret = device_property_read_u32(&pdev->dev,
>                                    "bus-frequency", &bus->bus_frequency);

Oh, please avoid double effort, i.e. go further and use I²C core APIs
for the timings. Oh, wait, do they use non-standard property?!

...

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

Personally I prefer using pointers in driver_data so we can avoid
ambiguity for the 0/NULL value returned by this call. But if 0 value
is considered invalid here, it's probably fine.

> +       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;


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 4/6] i2c: aspeed: Set the fwnode for the adap->dev
  2023-05-25 15:22 ` [RFC PATCH 4/6] i2c: aspeed: Set the fwnode for the adap->dev Jonathan Cameron
@ 2023-05-26 21:13   ` Andy Shevchenko
  0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2023-05-26 21:13 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-i2c, Wolfram Sang, Niyas Sait, Klaus Jensen,
	Andy Shevchenko, linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

On Thu, May 25, 2023 at 6:24 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> This is needed for the bus matching used for ACPI based
> i2c client registration.

...

> +       device_set_node(&bus->adap.dev, dev_fwnode(&pdev->dev));

Please, remove this
https://elixir.bootlin.com/linux/v6.4-rc1/source/drivers/i2c/busses/i2c-aspeed.c#L1029

at the same time.

>         ret = i2c_add_adapter(&bus->adap);
>         if (ret < 0)
>                 return ret;

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out.
  2023-05-25 15:22 ` [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out Jonathan Cameron
@ 2023-05-26 21:16   ` Andy Shevchenko
  2023-05-30 14:40     ` Jonathan Cameron
  2023-05-26 23:36   ` kernel test robot
  1 sibling, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2023-05-26 21:16 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-i2c, Wolfram Sang, Niyas Sait, Klaus Jensen,
	Andy Shevchenko, linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

On Thu, May 25, 2023 at 6:24 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> Needs tidying up - hopefully can do clock right
> using on going work from Niyas
> https://linaro.atlassian.net/wiki/spaces/CLIENTPC/pages/28832333867/ACPI+Clock+Management

For the current code base the easiest way is to switch to _optional
for clock, or request them based on the type of the fwnode. (Personal
preference is the _optional() API to call). For the reset isn't it
transparent already so we got a dummy control (as for regulator)?


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST
  2023-05-25 15:22 ` [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST Jonathan Cameron
  2023-05-26 10:37   ` kernel test robot
@ 2023-05-26 21:30   ` kernel test robot
  2023-05-30 14:44   ` Jonathan Cameron
  2 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2023-05-26 21:30 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: llvm, oe-kbuild-all

Hi Jonathan,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6]

url:    https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cameron/i2c-acpi-set-slave-mode-flag/20230525-233511
base:   f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
patch link:    https://lore.kernel.org/r/20230525152203.32190-7-Jonathan.Cameron%40huawei.com
patch subject: [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST
config: i386-randconfig-i013-20230526 (https://download.01.org/0day-ci/archive/20230527/202305270536.GXbPzREp-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/c866c2b49e19eaa618df6661c42b24b7506beba3
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jonathan-Cameron/i2c-acpi-set-slave-mode-flag/20230525-233511
        git checkout c866c2b49e19eaa618df6661c42b24b7506beba3
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/i2c/busses/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305270536.GXbPzREp-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/i2c/busses/i2c-aspeed.c:978:14: warning: unused variable 'parent_clk' [-Wunused-variable]
           struct clk *parent_clk;
                       ^
   1 warning generated.


vim +/parent_clk +978 drivers/i2c/busses/i2c-aspeed.c

87b59ff8d1d9d8 Brendan Higgins  2017-07-28   974  
f327c686d3ba44 Brendan Higgins  2017-06-20   975  static int aspeed_i2c_probe_bus(struct platform_device *pdev)
f327c686d3ba44 Brendan Higgins  2017-06-20   976  {
f327c686d3ba44 Brendan Higgins  2017-06-20   977  	struct aspeed_i2c_bus *bus;
f327c686d3ba44 Brendan Higgins  2017-06-20  @978  	struct clk *parent_clk;
f327c686d3ba44 Brendan Higgins  2017-06-20   979  	int irq, ret;
f327c686d3ba44 Brendan Higgins  2017-06-20   980  
f327c686d3ba44 Brendan Higgins  2017-06-20   981  	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
f327c686d3ba44 Brendan Higgins  2017-06-20   982  	if (!bus)
f327c686d3ba44 Brendan Higgins  2017-06-20   983  		return -ENOMEM;
f327c686d3ba44 Brendan Higgins  2017-06-20   984  
6b1e1925d82976 ye xingchen      2023-01-19   985  	bus->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
f327c686d3ba44 Brendan Higgins  2017-06-20   986  	if (IS_ERR(bus->base))
f327c686d3ba44 Brendan Higgins  2017-06-20   987  		return PTR_ERR(bus->base);
f327c686d3ba44 Brendan Higgins  2017-06-20   988  
d400cf85e735f5 Jonathan Cameron 2023-05-25   989  	//	parent_clk = devm_clk_get(&pdev->dev, NULL);
d400cf85e735f5 Jonathan Cameron 2023-05-25   990  	//	if (IS_ERR(parent_clk))//
d400cf85e735f5 Jonathan Cameron 2023-05-25   991  	//		return PTR_ERR(parent_clk);
d400cf85e735f5 Jonathan Cameron 2023-05-25   992  	bus->parent_clk_frequency = 1000000;//clk_get_rate(parent_clk);
f327c686d3ba44 Brendan Higgins  2017-06-20   993  	/* We just need the clock rate, we don't actually use the clk object. */
d400cf85e735f5 Jonathan Cameron 2023-05-25   994  	//devm_clk_put(&pdev->dev, parent_clk);
f327c686d3ba44 Brendan Higgins  2017-06-20   995  
edd20e95bca4a5 Joel Stanley     2017-11-01   996  	bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
edd20e95bca4a5 Joel Stanley     2017-11-01   997  	if (IS_ERR(bus->rst)) {
d400cf85e735f5 Jonathan Cameron 2023-05-25   998  		dev_warn(&pdev->dev,
6bc33c51976cd9 Jae Hyun Yoo     2018-07-02   999  			"missing or invalid reset controller device tree entry\n");
d400cf85e735f5 Jonathan Cameron 2023-05-25  1000  		bus->rst = 0;
d400cf85e735f5 Jonathan Cameron 2023-05-25  1001  	} else {
edd20e95bca4a5 Joel Stanley     2017-11-01  1002  		reset_control_deassert(bus->rst);
d400cf85e735f5 Jonathan Cameron 2023-05-25  1003  	}
edd20e95bca4a5 Joel Stanley     2017-11-01  1004  
248c1a4a98731d Jonathan Cameron 2023-05-25  1005  	ret = device_property_read_u32(&pdev->dev,
f327c686d3ba44 Brendan Higgins  2017-06-20  1006  				   "bus-frequency", &bus->bus_frequency);
f327c686d3ba44 Brendan Higgins  2017-06-20  1007  	if (ret < 0) {
f327c686d3ba44 Brendan Higgins  2017-06-20  1008  		dev_err(&pdev->dev,
f327c686d3ba44 Brendan Higgins  2017-06-20  1009  			"Could not read bus-frequency property\n");
90224e6468e15d Andy Shevchenko  2020-03-24  1010  		bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
f327c686d3ba44 Brendan Higgins  2017-06-20  1011  	}
f327c686d3ba44 Brendan Higgins  2017-06-20  1012  
17ccba67109cd0 Brendan Higgins  2018-09-21  1013  	bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
248c1a4a98731d Jonathan Cameron 2023-05-25  1014  		device_get_match_data(&pdev->dev);
248c1a4a98731d Jonathan Cameron 2023-05-25  1015  	if (!bus->get_clk_reg_val)
248c1a4a98731d Jonathan Cameron 2023-05-25  1016  		bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
87b59ff8d1d9d8 Brendan Higgins  2017-07-28  1017  
f327c686d3ba44 Brendan Higgins  2017-06-20  1018  	/* Initialize the I2C adapter */
f327c686d3ba44 Brendan Higgins  2017-06-20  1019  	spin_lock_init(&bus->lock);
f327c686d3ba44 Brendan Higgins  2017-06-20  1020  	init_completion(&bus->cmd_complete);
f327c686d3ba44 Brendan Higgins  2017-06-20  1021  	bus->adap.owner = THIS_MODULE;
f327c686d3ba44 Brendan Higgins  2017-06-20  1022  	bus->adap.retries = 0;
f327c686d3ba44 Brendan Higgins  2017-06-20  1023  	bus->adap.algo = &aspeed_i2c_algo;
f327c686d3ba44 Brendan Higgins  2017-06-20  1024  	bus->adap.dev.parent = &pdev->dev;
f327c686d3ba44 Brendan Higgins  2017-06-20  1025  	bus->adap.dev.of_node = pdev->dev.of_node;
ea1558ce149d28 Wolfram Sang     2022-08-11  1026  	strscpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
f327c686d3ba44 Brendan Higgins  2017-06-20  1027  	i2c_set_adapdata(&bus->adap, bus);
f327c686d3ba44 Brendan Higgins  2017-06-20  1028  
f327c686d3ba44 Brendan Higgins  2017-06-20  1029  	bus->dev = &pdev->dev;
f327c686d3ba44 Brendan Higgins  2017-06-20  1030  
f327c686d3ba44 Brendan Higgins  2017-06-20  1031  	/* Clean up any left over interrupt state. */
f327c686d3ba44 Brendan Higgins  2017-06-20  1032  	writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
f327c686d3ba44 Brendan Higgins  2017-06-20  1033  	writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
f327c686d3ba44 Brendan Higgins  2017-06-20  1034  	/*
f327c686d3ba44 Brendan Higgins  2017-06-20  1035  	 * bus.lock does not need to be held because the interrupt handler has
f327c686d3ba44 Brendan Higgins  2017-06-20  1036  	 * not been enabled yet.
f327c686d3ba44 Brendan Higgins  2017-06-20  1037  	 */
f327c686d3ba44 Brendan Higgins  2017-06-20  1038  	ret = aspeed_i2c_init(bus, pdev);
f327c686d3ba44 Brendan Higgins  2017-06-20  1039  	if (ret < 0)
f327c686d3ba44 Brendan Higgins  2017-06-20  1040  		return ret;
f327c686d3ba44 Brendan Higgins  2017-06-20  1041  
f2016870518449 Jonathan Cameron 2023-05-25  1042  	irq = platform_get_irq(pdev, 0);
f327c686d3ba44 Brendan Higgins  2017-06-20  1043  	ret = devm_request_irq(&pdev->dev, irq, aspeed_i2c_bus_irq,
f327c686d3ba44 Brendan Higgins  2017-06-20  1044  			       0, dev_name(&pdev->dev), bus);
f327c686d3ba44 Brendan Higgins  2017-06-20  1045  	if (ret < 0)
f327c686d3ba44 Brendan Higgins  2017-06-20  1046  		return ret;
f327c686d3ba44 Brendan Higgins  2017-06-20  1047  
348e97f7dd4ae4 Jonathan Cameron 2023-05-25  1048  	device_set_node(&bus->adap.dev, dev_fwnode(&pdev->dev));
348e97f7dd4ae4 Jonathan Cameron 2023-05-25  1049  
f327c686d3ba44 Brendan Higgins  2017-06-20  1050  	ret = i2c_add_adapter(&bus->adap);
f327c686d3ba44 Brendan Higgins  2017-06-20  1051  	if (ret < 0)
f327c686d3ba44 Brendan Higgins  2017-06-20  1052  		return ret;
f327c686d3ba44 Brendan Higgins  2017-06-20  1053  
f327c686d3ba44 Brendan Higgins  2017-06-20  1054  	platform_set_drvdata(pdev, bus);
f327c686d3ba44 Brendan Higgins  2017-06-20  1055  
f327c686d3ba44 Brendan Higgins  2017-06-20  1056  	dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
f327c686d3ba44 Brendan Higgins  2017-06-20  1057  		 bus->adap.nr, irq);
f327c686d3ba44 Brendan Higgins  2017-06-20  1058  
f327c686d3ba44 Brendan Higgins  2017-06-20  1059  	return 0;
f327c686d3ba44 Brendan Higgins  2017-06-20  1060  }
f327c686d3ba44 Brendan Higgins  2017-06-20  1061  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out.
  2023-05-25 15:22 ` [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out Jonathan Cameron
  2023-05-26 21:16   ` Andy Shevchenko
@ 2023-05-26 23:36   ` kernel test robot
  1 sibling, 0 replies; 19+ messages in thread
From: kernel test robot @ 2023-05-26 23:36 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: oe-kbuild-all

Hi Jonathan,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6]

url:    https://github.com/intel-lab-lkp/linux/commits/Jonathan-Cameron/i2c-acpi-set-slave-mode-flag/20230525-233511
base:   f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
patch link:    https://lore.kernel.org/r/20230525152203.32190-6-Jonathan.Cameron%40huawei.com
patch subject: [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out.
config: mips-randconfig-s042-20230526 (https://download.01.org/0day-ci/archive/20230527/202305270648.LyxZ5g9R-lkp@intel.com/config)
compiler: mipsel-linux-gcc (GCC) 12.1.0
reproduce:
        mkdir -p ~/bin
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/d400cf85e735f50ef4afd3c3aabf377926a6506a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jonathan-Cameron/i2c-acpi-set-slave-mode-flag/20230525-233511
        git checkout d400cf85e735f50ef4afd3c3aabf377926a6506a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 ~/bin/make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=mips SHELL=/bin/bash drivers/i2c/busses/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202305270648.LyxZ5g9R-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/i2c/busses/i2c-aspeed.c:1000:28: sparse: sparse: Using plain integer as NULL pointer

vim +1000 drivers/i2c/busses/i2c-aspeed.c

   974	
   975	static int aspeed_i2c_probe_bus(struct platform_device *pdev)
   976	{
   977		struct aspeed_i2c_bus *bus;
   978		struct clk *parent_clk;
   979		int irq, ret;
   980	
   981		bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
   982		if (!bus)
   983			return -ENOMEM;
   984	
   985		bus->base = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
   986		if (IS_ERR(bus->base))
   987			return PTR_ERR(bus->base);
   988	
   989		//	parent_clk = devm_clk_get(&pdev->dev, NULL);
   990		//	if (IS_ERR(parent_clk))//
   991		//		return PTR_ERR(parent_clk);
   992		bus->parent_clk_frequency = 1000000;//clk_get_rate(parent_clk);
   993		/* We just need the clock rate, we don't actually use the clk object. */
   994		//devm_clk_put(&pdev->dev, parent_clk);
   995	
   996		bus->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
   997		if (IS_ERR(bus->rst)) {
   998			dev_warn(&pdev->dev,
   999				"missing or invalid reset controller device tree entry\n");
> 1000			bus->rst = 0;
  1001		} else {
  1002			reset_control_deassert(bus->rst);
  1003		}
  1004	
  1005		ret = device_property_read_u32(&pdev->dev,
  1006					   "bus-frequency", &bus->bus_frequency);
  1007		if (ret < 0) {
  1008			dev_err(&pdev->dev,
  1009				"Could not read bus-frequency property\n");
  1010			bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
  1011		}
  1012	
  1013		bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
  1014			device_get_match_data(&pdev->dev);
  1015		if (!bus->get_clk_reg_val)
  1016			bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
  1017	
  1018		/* Initialize the I2C adapter */
  1019		spin_lock_init(&bus->lock);
  1020		init_completion(&bus->cmd_complete);
  1021		bus->adap.owner = THIS_MODULE;
  1022		bus->adap.retries = 0;
  1023		bus->adap.algo = &aspeed_i2c_algo;
  1024		bus->adap.dev.parent = &pdev->dev;
  1025		bus->adap.dev.of_node = pdev->dev.of_node;
  1026		strscpy(bus->adap.name, pdev->name, sizeof(bus->adap.name));
  1027		i2c_set_adapdata(&bus->adap, bus);
  1028	
  1029		bus->dev = &pdev->dev;
  1030	
  1031		/* Clean up any left over interrupt state. */
  1032		writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
  1033		writel(0xffffffff, bus->base + ASPEED_I2C_INTR_STS_REG);
  1034		/*
  1035		 * bus.lock does not need to be held because the interrupt handler has
  1036		 * not been enabled yet.
  1037		 */
  1038		ret = aspeed_i2c_init(bus, pdev);
  1039		if (ret < 0)
  1040			return ret;
  1041	
  1042		irq = platform_get_irq(pdev, 0);
  1043		ret = devm_request_irq(&pdev->dev, irq, aspeed_i2c_bus_irq,
  1044				       0, dev_name(&pdev->dev), bus);
  1045		if (ret < 0)
  1046			return ret;
  1047	
  1048		device_set_node(&bus->adap.dev, dev_fwnode(&pdev->dev));
  1049	
  1050		ret = i2c_add_adapter(&bus->adap);
  1051		if (ret < 0)
  1052			return ret;
  1053	
  1054		platform_set_drvdata(pdev, bus);
  1055	
  1056		dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
  1057			 bus->adap.nr, irq);
  1058	
  1059		return 0;
  1060	}
  1061	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 3/6] i2c: aspeed: switch to generic fw properties.
  2023-05-26 21:11   ` Andy Shevchenko
@ 2023-05-30 14:16     ` Jonathan Cameron
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-30 14:16 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-i2c, Wolfram Sang, Niyas Sait, Klaus Jensen,
	Andy Shevchenko, linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

On Sat, 27 May 2023 00:11:09 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, May 25, 2023 at 6:23 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > Not tested on device tree but works nicely for ACPI :)  

I was planning to abandon these as 'on list for anyone who
cared' but now you've reviewed them I guess I better do
an RFC v2 :)

> 
> Needs a better commit message obviously :-)

:)

> 
> ...
> 
> > -       ret = of_property_read_u32(pdev->dev.of_node,
> > +       ret = device_property_read_u32(&pdev->dev,
> >                                    "bus-frequency", &bus->bus_frequency);  
> 
> Oh, please avoid double effort, i.e. go further and use I²C core APIs
> for the timings. Oh, wait, do they use non-standard property?!

yup :(

Though it is documented as having a default of 100kHz in the devicetree
binding so the original code shouldn't be calling dev_err() and should
just do:

	bus->frequency = I2C_MAX_STANDARD_MODE_FREQ;
	device_property_read_u32(&pdev->dev,
				 "bus-frequency, &bus->frequency);

Fixing that is an unrelated change though. I'll do it for dt
in a precusor patch then carry that forward to here.

> 
> ...
> 
> > +       bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
> > +               device_get_match_data(&pdev->dev);  
> 
> Personally I prefer using pointers in driver_data so we can avoid
> ambiguity for the 0/NULL value returned by this call. But if 0 value
> is considered invalid here, it's probably fine.

It is a pointer, just a function pointer rather than to a structure.
I could wrap it up in a structure but that would be an unrelated
driver change so at very least a separate patch. 

> 
> > +       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;  
> 
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out.
  2023-05-26 21:16   ` Andy Shevchenko
@ 2023-05-30 14:40     ` Jonathan Cameron
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-30 14:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-i2c, Wolfram Sang, Niyas Sait, Klaus Jensen,
	Andy Shevchenko, linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl, linuxarm,
	Viacheslav A . Dubeyko

On Sat, 27 May 2023 00:16:36 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, May 25, 2023 at 6:24 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > Needs tidying up - hopefully can do clock right
> > using on going work from Niyas
> > https://linaro.atlassian.net/wiki/spaces/CLIENTPC/pages/28832333867/ACPI+Clock+Management  
> 
> For the current code base the easiest way is to switch to _optional
> for clock, or request them based on the type of the fwnode. (Personal
> preference is the _optional() API to call). 

Absolutely agree that would the way to go if people want to support my
crazy.

However, that will leave the input clock frequency unknown which means we'll
program a garbage value into one of the device registers. Doesn't matter
to me, but not good in general.

This is avoiding for now the questions of:
1) Why devm for a clock we hold for 2 lines of code, none of which
   have an error return path...
2) clk_get_rate() is documented as not guaranteed to do anything for
   a clk until enabled, so this is relying on it being enabled by
   someone else or a quirk of the the chip. 

> For the reset isn't it
> transparent already so we got a dummy control (as for regulator)?

I don't think so, but maybe I'm missing something.
There is a devm_reset_control_get_optional() though, similar to the clock
one that returns a NULL if not present. 

I'll use that here to make this a slightly less ugly hack.
If I can handle clocks nicely using Niyas' work then can revisit
whether the i2c and aspeed maintainers would accept making the
reset optional.

Jonathan


> 
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST
  2023-05-25 15:22 ` [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST Jonathan Cameron
  2023-05-26 10:37   ` kernel test robot
  2023-05-26 21:30   ` kernel test robot
@ 2023-05-30 14:44   ` Jonathan Cameron
  2 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2023-05-30 14:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Niyas Sait, Klaus Jensen, Andy Shevchenko,
	linux-acpi, Jeremy Kerr, Matt Johnston,
	Shesha Bhushan Sreenivasamurthy, linux-cxl,
	Viacheslav A . Dubeyko

On Thu, 25 May 2023 16:22:03 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> See cover letter for why...
> This works nicely on x86 with appropriate evil emulation.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
I'll drop this from v2. It's obviously unnecessary. I'll
just tell people who care to enable COMPILE_TEST
We shouldn't carry my lazy hack around ;)

Jonathan

> ---
>  drivers/i2c/busses/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 87600b4aacb3..96bb5a05e195 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -391,7 +391,6 @@ config I2C_ALTERA
>  
>  config I2C_ASPEED
>  	tristate "Aspeed I2C Controller"
> -	depends on ARCH_ASPEED || COMPILE_TEST
>  	help
>  	  If you say yes to this option, support will be included for the
>  	  Aspeed I2C controller.


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-05-30 14:44 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-25 15:21 [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
2023-05-25 15:21 ` [RFC PATCH 1/6] i2c: acpi: set slave mode flag Jonathan Cameron
2023-05-26 19:43   ` Andy Shevchenko
2023-05-25 15:21 ` [RFC PATCH 2/6] i2c: aspeed: Use platform_get_irq() instead of opencoding Jonathan Cameron
2023-05-26 21:06   ` Andy Shevchenko
2023-05-25 15:22 ` [RFC PATCH 3/6] i2c: aspeed: switch to generic fw properties Jonathan Cameron
2023-05-26 21:11   ` Andy Shevchenko
2023-05-30 14:16     ` Jonathan Cameron
2023-05-25 15:22 ` [RFC PATCH 4/6] i2c: aspeed: Set the fwnode for the adap->dev Jonathan Cameron
2023-05-26 21:13   ` Andy Shevchenko
2023-05-25 15:22 ` [RFC PATCH 5/6] HACK: i2c: aspeed: Comment the clock and reset out Jonathan Cameron
2023-05-26 21:16   ` Andy Shevchenko
2023-05-30 14:40     ` Jonathan Cameron
2023-05-26 23:36   ` kernel test robot
2023-05-25 15:22 ` [RFC PATCH 6/6] HACK: i2c: aspeed: Enable build without COMPILE_TEST Jonathan Cameron
2023-05-26 10:37   ` kernel test robot
2023-05-26 21:30   ` kernel test robot
2023-05-30 14:44   ` Jonathan Cameron
2023-05-25 16:18 ` [RFC PATCH 0/6] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.