linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI
@ 2023-05-31 10:05 Jonathan Cameron
  2023-05-31 10:05 ` [RFC PATCH v3 1/7] i2c: acpi: set slave mode flag Jonathan Cameron
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-31 10:05 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

v3:
 - More improvements from Andy, called out in the individual patches.

Obviously I am sending this much quicker than would normally make sense.

Whilst Andy was kind enough to provide review I never really expected
anyone to look at these in depth, I just needed them public so I could
tell people to use them. However I don't want to loose the improvements
from Andy's review.

Hence still RFC though if people like the first 6 patches, they may be
worth picking up anyway.

I'm planning to forget about these entirely for a few weeks at least
and work on the other parts of the stack this enables and a general
huge backlog of things I've been ignoring.

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 6 patches might be suitable for upstream inclusion, the
last one - 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 (7):
  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: use a function pointer type def for clk_reg_val callback
  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 | 48 +++++++++++++++------------------
 drivers/i2c/i2c-core-acpi.c     |  3 +++
 2 files changed, 25 insertions(+), 26 deletions(-)

-- 
2.39.2


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

* [RFC PATCH v3 1/7] i2c: acpi: set slave mode flag
  2023-05-31 10:05 [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
@ 2023-05-31 10:05 ` Jonathan Cameron
  2023-05-31 10:05 ` [RFC PATCH v3 2/7] i2c: aspeed: Use platform_get_irq() instead of opencoding Jonathan Cameron
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-31 10:05 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.

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


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

* [RFC PATCH v3 2/7] i2c: aspeed: Use platform_get_irq() instead of opencoding
  2023-05-31 10:05 [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
  2023-05-31 10:05 ` [RFC PATCH v3 1/7] i2c: acpi: set slave mode flag Jonathan Cameron
@ 2023-05-31 10:05 ` Jonathan Cameron
  2023-05-31 17:41   ` Andy Shevchenko
  2023-05-31 10:05 ` [RFC PATCH v3 3/7] i2c: aspeed: Don't report error when optional dt bus-frequency not supplied Jonathan Cameron
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-31 10:05 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 its 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>

---
v3: Fix it's -> its in description.
---
 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


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

* [RFC PATCH v3 3/7] i2c: aspeed: Don't report error when optional dt bus-frequency not supplied
  2023-05-31 10:05 [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
  2023-05-31 10:05 ` [RFC PATCH v3 1/7] i2c: acpi: set slave mode flag Jonathan Cameron
  2023-05-31 10:05 ` [RFC PATCH v3 2/7] i2c: aspeed: Use platform_get_irq() instead of opencoding Jonathan Cameron
@ 2023-05-31 10:05 ` Jonathan Cameron
  2023-05-31 10:05 ` [RFC PATCH v3 4/7] i2c: aspeed: use a function pointer type def for clk_reg_val callback Jonathan Cameron
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-31 10:05 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

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.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 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


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

* [RFC PATCH v3 4/7] i2c: aspeed: use a function pointer type def for clk_reg_val callback
  2023-05-31 10:05 [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
                   ` (2 preceding siblings ...)
  2023-05-31 10:05 ` [RFC PATCH v3 3/7] i2c: aspeed: Don't report error when optional dt bus-frequency not supplied Jonathan Cameron
@ 2023-05-31 10:05 ` Jonathan Cameron
  2023-05-31 17:42   ` Andy Shevchenko
  2023-05-31 10:05 ` [RFC PATCH v3 5/7] i2c: aspeed: switch to generic fw properties Jonathan Cameron
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-31 10:05 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

Rather than having to define the parameter types of this function
in multiple places, use a single typedef.

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

---
v3: New patch to switch to a function pointer as suggested by Andy.
---
 drivers/i2c/busses/i2c-aspeed.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 4363bfe06f9b..be93de56f7e4 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -137,6 +137,8 @@ enum aspeed_i2c_slave_state {
 	ASPEED_I2C_SLAVE_STOP,
 };
 
+typedef u32 (*aspeed_get_clk_reg_val_cb)(struct device *dev, u32 divisor);
+
 struct aspeed_i2c_bus {
 	struct i2c_adapter		adap;
 	struct device			*dev;
@@ -145,8 +147,7 @@ struct aspeed_i2c_bus {
 	/* Synchronizes I/O mem access to base. */
 	spinlock_t			lock;
 	struct completion		cmd_complete;
-	u32				(*get_clk_reg_val)(struct device *dev,
-							   u32 divisor);
+	aspeed_get_clk_reg_val_cb	get_clk_reg_val;
 	unsigned long			parent_clk_frequency;
 	u32				bus_frequency;
 	/* Transaction state. */
@@ -1011,8 +1012,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
 	if (!match)
 		bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
 	else
-		bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
-				match->data;
+		bus->get_clk_reg_val = (aspeed_get_clk_reg_val_cb)(match->data);
 
 	/* Initialize the I2C adapter */
 	spin_lock_init(&bus->lock);
-- 
2.39.2


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

* [RFC PATCH v3 5/7] i2c: aspeed: switch to generic fw properties.
  2023-05-31 10:05 [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
                   ` (3 preceding siblings ...)
  2023-05-31 10:05 ` [RFC PATCH v3 4/7] i2c: aspeed: use a function pointer type def for clk_reg_val callback Jonathan Cameron
@ 2023-05-31 10:05 ` Jonathan Cameron
  2023-05-31 17:45   ` Andy Shevchenko
  2023-05-31 10:05 ` [RFC PATCH v3 6/7] i2c: aspeed: Set the fwnode for the adap->dev Jonathan Cameron
  2023-05-31 10:06 ` [RFC PATCH v3 7/7] HACK: i2c: aspeed: Comment clock out and make reset optional Jonathan Cameron
  6 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-31 10:05 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

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>

---
v3:
- Use a typedef for the function pointer type as introduced in precusor
  patch
---
 drivers/i2c/busses/i2c-aspeed.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index be93de56f7e4..992d64acd38d 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>
 
@@ -976,7 +975,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;
@@ -1005,14 +1003,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 =
+		(aspeed_get_clk_reg_val_cb)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 = (aspeed_get_clk_reg_val_cb)(match->data);
 
 	/* Initialize the I2C adapter */
 	spin_lock_init(&bus->lock);
-- 
2.39.2


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

* [RFC PATCH v3 6/7] i2c: aspeed: Set the fwnode for the adap->dev
  2023-05-31 10:05 [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
                   ` (4 preceding siblings ...)
  2023-05-31 10:05 ` [RFC PATCH v3 5/7] i2c: aspeed: switch to generic fw properties Jonathan Cameron
@ 2023-05-31 10:05 ` Jonathan Cameron
  2023-05-31 17:46   ` Andy Shevchenko
  2023-05-31 10:06 ` [RFC PATCH v3 7/7] HACK: i2c: aspeed: Comment clock out and make reset optional Jonathan Cameron
  6 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-31 10:05 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.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 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 992d64acd38d..e262b06e224b 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -1018,7 +1018,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);
 
@@ -1044,6 +1043,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] 15+ messages in thread

* [RFC PATCH v3 7/7] HACK: i2c: aspeed: Comment clock out and make reset optional
  2023-05-31 10:05 [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
                   ` (5 preceding siblings ...)
  2023-05-31 10:05 ` [RFC PATCH v3 6/7] i2c: aspeed: Set the fwnode for the adap->dev Jonathan Cameron
@ 2023-05-31 10:06 ` Jonathan Cameron
  6 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-05-31 10:06 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

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>
---
v3: Keeping this as an obvious hack rather than moving to something
    that works in the interest of not putting a stop gap solution in
    place.
---
 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 e262b06e224b..e29e7effd911 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -987,14 +987,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


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

* Re: [RFC PATCH v3 2/7] i2c: aspeed: Use platform_get_irq() instead of opencoding
  2023-05-31 10:05 ` [RFC PATCH v3 2/7] i2c: aspeed: Use platform_get_irq() instead of opencoding Jonathan Cameron
@ 2023-05-31 17:41   ` Andy Shevchenko
  0 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2023-05-31 17:41 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 Wed, May 31, 2023 at 1:07 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> A cleanup in its 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)

Hadn't I provided a tag?
Anyway,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> ---
> v3: Fix it's -> its in description.
> ---
>  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
>


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [RFC PATCH v3 4/7] i2c: aspeed: use a function pointer type def for clk_reg_val callback
  2023-05-31 10:05 ` [RFC PATCH v3 4/7] i2c: aspeed: use a function pointer type def for clk_reg_val callback Jonathan Cameron
@ 2023-05-31 17:42   ` Andy Shevchenko
  2023-06-01  9:07     ` Jonathan Cameron
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2023-05-31 17:42 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 Wed, May 31, 2023 at 1:08 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> Rather than having to define the parameter types of this function
> in multiple places, use a single typedef.

Suggested-by then?

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

> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> ---
> v3: New patch to switch to a function pointer as suggested by Andy.
> ---
>  drivers/i2c/busses/i2c-aspeed.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index 4363bfe06f9b..be93de56f7e4 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -137,6 +137,8 @@ enum aspeed_i2c_slave_state {
>         ASPEED_I2C_SLAVE_STOP,
>  };
>
> +typedef u32 (*aspeed_get_clk_reg_val_cb)(struct device *dev, u32 divisor);
> +
>  struct aspeed_i2c_bus {
>         struct i2c_adapter              adap;
>         struct device                   *dev;
> @@ -145,8 +147,7 @@ struct aspeed_i2c_bus {
>         /* Synchronizes I/O mem access to base. */
>         spinlock_t                      lock;
>         struct completion               cmd_complete;
> -       u32                             (*get_clk_reg_val)(struct device *dev,
> -                                                          u32 divisor);
> +       aspeed_get_clk_reg_val_cb       get_clk_reg_val;
>         unsigned long                   parent_clk_frequency;
>         u32                             bus_frequency;
>         /* Transaction state. */
> @@ -1011,8 +1012,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>         if (!match)
>                 bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
>         else
> -               bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
> -                               match->data;
> +               bus->get_clk_reg_val = (aspeed_get_clk_reg_val_cb)(match->data);
>
>         /* Initialize the I2C adapter */
>         spin_lock_init(&bus->lock);
> --
> 2.39.2
>


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [RFC PATCH v3 5/7] i2c: aspeed: switch to generic fw properties.
  2023-05-31 10:05 ` [RFC PATCH v3 5/7] i2c: aspeed: switch to generic fw properties Jonathan Cameron
@ 2023-05-31 17:45   ` Andy Shevchenko
  2023-06-01  9:08     ` Jonathan Cameron
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2023-05-31 17:45 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 Wed, May 31, 2023 at 1:08 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.

...

>  static int aspeed_i2c_probe_bus(struct platform_device *pdev)
>  {
> -       const struct of_device_id *match;

With

  struct device *dev = &pdev->dev;

...

> +       device_property_read_u32(&pdev->dev,
> +                                "bus-frequency", &bus->bus_frequency);

This can take one or both parameters on one line.

...

> +       bus->get_clk_reg_val =
> +               (aspeed_get_clk_reg_val_cb)device_get_match_data(&pdev->dev);

This one as well I believe.

Also others, but it can be done in a separate patch.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [RFC PATCH v3 6/7] i2c: aspeed: Set the fwnode for the adap->dev
  2023-05-31 10:05 ` [RFC PATCH v3 6/7] i2c: aspeed: Set the fwnode for the adap->dev Jonathan Cameron
@ 2023-05-31 17:46   ` Andy Shevchenko
  2023-06-01  9:10     ` Jonathan Cameron
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2023-05-31 17:46 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 Wed, May 31, 2023 at 1:09 PM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> This is needed for the bus matching used for ACPI based
> i2c client registration.

Btw, this patch like maybe others (e.g., patch 1) can be sent already
as non-RFC and applied independently on the goal of the entire series.

-- 
With Best Regards,
Andy Shevchenko

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

* Re: [RFC PATCH v3 4/7] i2c: aspeed: use a function pointer type def for clk_reg_val callback
  2023-05-31 17:42   ` Andy Shevchenko
@ 2023-06-01  9:07     ` Jonathan Cameron
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-06-01  9:07 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 Wed, 31 May 2023 20:42:15 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, May 31, 2023 at 1:08 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > Rather than having to define the parameter types of this function
> > in multiple places, use a single typedef.  
> 
> Suggested-by then?
> 
Good point.

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

Hopefully Wolfram uses b4 and that will get picked up automatically.

> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > ---
> > v3: New patch to switch to a function pointer as suggested by Andy.
> > ---
> >  drivers/i2c/busses/i2c-aspeed.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> > index 4363bfe06f9b..be93de56f7e4 100644
> > --- a/drivers/i2c/busses/i2c-aspeed.c
> > +++ b/drivers/i2c/busses/i2c-aspeed.c
> > @@ -137,6 +137,8 @@ enum aspeed_i2c_slave_state {
> >         ASPEED_I2C_SLAVE_STOP,
> >  };
> >
> > +typedef u32 (*aspeed_get_clk_reg_val_cb)(struct device *dev, u32 divisor);
> > +
> >  struct aspeed_i2c_bus {
> >         struct i2c_adapter              adap;
> >         struct device                   *dev;
> > @@ -145,8 +147,7 @@ struct aspeed_i2c_bus {
> >         /* Synchronizes I/O mem access to base. */
> >         spinlock_t                      lock;
> >         struct completion               cmd_complete;
> > -       u32                             (*get_clk_reg_val)(struct device *dev,
> > -                                                          u32 divisor);
> > +       aspeed_get_clk_reg_val_cb       get_clk_reg_val;
> >         unsigned long                   parent_clk_frequency;
> >         u32                             bus_frequency;
> >         /* Transaction state. */
> > @@ -1011,8 +1012,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> >         if (!match)
> >                 bus->get_clk_reg_val = aspeed_i2c_24xx_get_clk_reg_val;
> >         else
> > -               bus->get_clk_reg_val = (u32 (*)(struct device *, u32))
> > -                               match->data;
> > +               bus->get_clk_reg_val = (aspeed_get_clk_reg_val_cb)(match->data);
> >
> >         /* Initialize the I2C adapter */
> >         spin_lock_init(&bus->lock);
> > --
> > 2.39.2
> >  
> 
> 


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

* Re: [RFC PATCH v3 5/7] i2c: aspeed: switch to generic fw properties.
  2023-05-31 17:45   ` Andy Shevchenko
@ 2023-06-01  9:08     ` Jonathan Cameron
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-06-01  9:08 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 Wed, 31 May 2023 20:45:08 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, May 31, 2023 at 1:08 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.  
> 
> ...
> 
> >  static int aspeed_i2c_probe_bus(struct platform_device *pdev)
> >  {
> > -       const struct of_device_id *match;  
> 
> With
> 
>   struct device *dev = &pdev->dev;
> 
> ...
> 
> > +       device_property_read_u32(&pdev->dev,
> > +                                "bus-frequency", &bus->bus_frequency);  
> 
> This can take one or both parameters on one line.
> 
> ...
> 
> > +       bus->get_clk_reg_val =
> > +               (aspeed_get_clk_reg_val_cb)device_get_match_data(&pdev->dev);  
> 
> This one as well I believe.
> 
> Also others, but it can be done in a separate patch.
> 

I thought about it, but decided out of scope for this set.
I'm not aiming for too much general tidying!

Jonathan



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

* Re: [RFC PATCH v3 6/7] i2c: aspeed: Set the fwnode for the adap->dev
  2023-05-31 17:46   ` Andy Shevchenko
@ 2023-06-01  9:10     ` Jonathan Cameron
  0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2023-06-01  9:10 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 Wed, 31 May 2023 20:46:42 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Wed, May 31, 2023 at 1:09 PM Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > This is needed for the bus matching used for ACPI based
> > i2c client registration.  
> 
> Btw, this patch like maybe others (e.g., patch 1) can be sent already
> as non-RFC and applied independently on the goal of the entire series.
> 

True but then I have to reference two sets of precursors for
what I care about for now.  Now this is fairly stable, I'll circle
back to post these as a separate set - I can carry on referencing the
RFC from the QEMU side of things (and blog post etc I should write on this...)

Jonathan

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

end of thread, other threads:[~2023-06-01  9:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 10:05 [RFC PATCH v3 0/7] i2c: Enabling use of aspeed-i2c with ACPI Jonathan Cameron
2023-05-31 10:05 ` [RFC PATCH v3 1/7] i2c: acpi: set slave mode flag Jonathan Cameron
2023-05-31 10:05 ` [RFC PATCH v3 2/7] i2c: aspeed: Use platform_get_irq() instead of opencoding Jonathan Cameron
2023-05-31 17:41   ` Andy Shevchenko
2023-05-31 10:05 ` [RFC PATCH v3 3/7] i2c: aspeed: Don't report error when optional dt bus-frequency not supplied Jonathan Cameron
2023-05-31 10:05 ` [RFC PATCH v3 4/7] i2c: aspeed: use a function pointer type def for clk_reg_val callback Jonathan Cameron
2023-05-31 17:42   ` Andy Shevchenko
2023-06-01  9:07     ` Jonathan Cameron
2023-05-31 10:05 ` [RFC PATCH v3 5/7] i2c: aspeed: switch to generic fw properties Jonathan Cameron
2023-05-31 17:45   ` Andy Shevchenko
2023-06-01  9:08     ` Jonathan Cameron
2023-05-31 10:05 ` [RFC PATCH v3 6/7] i2c: aspeed: Set the fwnode for the adap->dev Jonathan Cameron
2023-05-31 17:46   ` Andy Shevchenko
2023-06-01  9:10     ` Jonathan Cameron
2023-05-31 10:06 ` [RFC PATCH v3 7/7] HACK: i2c: aspeed: Comment clock out and make reset optional Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).