Recently ACPI gained the acpi_get_local_address() API which may be used
instead of home grown i2c_mux_gpio_get_acpi_adr().
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/i2c/muxes/i2c-mux-gpio.c | 43 ++------------------------------
1 file changed, 2 insertions(+), 41 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index bac415a52b78..31e6eb1591bb 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -49,45 +49,6 @@ static int i2c_mux_gpio_deselect(struct i2c_mux_core *muxc, u32 chan)
return 0;
}
-#ifdef CONFIG_ACPI
-
-static int i2c_mux_gpio_get_acpi_adr(struct device *dev,
- struct fwnode_handle *fwdev,
- unsigned int *adr)
-
-{
- unsigned long long adr64;
- acpi_status status;
-
- status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(fwdev),
- METHOD_NAME__ADR,
- NULL, &adr64);
-
- if (!ACPI_SUCCESS(status)) {
- dev_err(dev, "Cannot get address\n");
- return -EINVAL;
- }
-
- *adr = adr64;
- if (*adr != adr64) {
- dev_err(dev, "Address out of range\n");
- return -ERANGE;
- }
-
- return 0;
-}
-
-#else
-
-static int i2c_mux_gpio_get_acpi_adr(struct device *dev,
- struct fwnode_handle *fwdev,
- unsigned int *adr)
-{
- return -EINVAL;
-}
-
-#endif
-
static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
struct platform_device *pdev)
{
@@ -141,9 +102,9 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
fwnode_property_read_u32(child, "reg", values + i);
} else if (is_acpi_node(child)) {
- rc = i2c_mux_gpio_get_acpi_adr(dev, child, values + i);
+ rc = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), values + i);
if (rc)
- return rc;
+ return dev_err_probe(dev, rc, "Cannot get address\n");
}
i++;
--
2.33.0
Use array_size() helper to aid in 2-factor allocation instances.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/i2c/muxes/i2c-mux-gpio.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index b09c10f36ddb..73a23e117ebe 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -7,6 +7,7 @@
#include <linux/i2c.h>
#include <linux/i2c-mux.h>
+#include <linux/overflow.h>
#include <linux/platform_data/i2c-mux-gpio.h>
#include <linux/platform_device.h>
#include <linux/module.h>
@@ -152,7 +153,7 @@ static int i2c_mux_gpio_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
muxc = i2c_mux_alloc(parent, &pdev->dev, mux->data.n_values,
- ngpios * sizeof(*mux->gpios), 0,
+ array_size(ngpios, sizeof(*mux->gpios)), 0,
i2c_mux_gpio_select, NULL);
if (!muxc) {
ret = -ENOMEM;
--
2.33.0
We have a special helper to get fwnode out of struct device.
Moreover, dereferencing it directly prevents the fwnode
modifications in the future.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/i2c/muxes/i2c-mux-gpio.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 31e6eb1591bb..b09c10f36ddb 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -53,6 +53,7 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
struct device_node *np = dev->of_node;
struct device_node *adapter_np;
struct i2c_adapter *adapter = NULL;
@@ -60,7 +61,7 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
unsigned *values;
int rc, i = 0;
- if (is_of_node(dev->fwnode)) {
+ if (is_of_node(fwnode)) {
if (!np)
return -ENODEV;
@@ -72,7 +73,7 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
adapter = of_find_i2c_adapter_by_node(adapter_np);
of_node_put(adapter_np);
- } else if (is_acpi_node(dev->fwnode)) {
+ } else if (is_acpi_node(fwnode)) {
/*
* In ACPI land the mux should be a direct child of the i2c
* bus it muxes.
@@ -111,7 +112,7 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
}
mux->data.values = values;
- if (fwnode_property_read_u32(dev->fwnode, "idle-state", &mux->data.idle))
+ if (device_property_read_u32(dev, "idle-state", &mux->data.idle))
mux->data.idle = I2C_MUX_GPIO_NO_IDLE;
return 0;
--
2.33.0
On Mon, Nov 15, 2021 at 7:42 AM Andy Shevchenko
<[email protected]> wrote:
>
> Recently ACPI gained the acpi_get_local_address() API which may be used
> instead of home grown i2c_mux_gpio_get_acpi_adr().
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Evan Green <[email protected]>
On Mon, Nov 15, 2021 at 7:42 AM Andy Shevchenko
<[email protected]> wrote:
>
> We have a special helper to get fwnode out of struct device.
> Moreover, dereferencing it directly prevents the fwnode
> modifications in the future.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Reviewed-by: Evan Green <[email protected]>
On Mon, Nov 15, 2021 at 7:42 AM Andy Shevchenko
<[email protected]> wrote:
>
> Use array_size() helper to aid in 2-factor allocation instances.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Thanks for the cleanup series!
Reviewed-by: Evan Green <[email protected]>
Hi!
On 2021-11-15 16:41, Andy Shevchenko wrote:
> Recently ACPI gained the acpi_get_local_address() API which may be used
> instead of home grown i2c_mux_gpio_get_acpi_adr().
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/i2c/muxes/i2c-mux-gpio.c | 43 ++------------------------------
> 1 file changed, 2 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
> index bac415a52b78..31e6eb1591bb 100644
> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
> @@ -49,45 +49,6 @@ static int i2c_mux_gpio_deselect(struct i2c_mux_core *muxc, u32 chan)
> return 0;
> }
>
> -#ifdef CONFIG_ACPI
> -
> -static int i2c_mux_gpio_get_acpi_adr(struct device *dev,
> - struct fwnode_handle *fwdev,
> - unsigned int *adr)
> -
> -{
> - unsigned long long adr64;
> - acpi_status status;
> -
> - status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(fwdev),
> - METHOD_NAME__ADR,
> - NULL, &adr64);
> -
> - if (!ACPI_SUCCESS(status)) {
> - dev_err(dev, "Cannot get address\n");
> - return -EINVAL;
> - }
> -
> - *adr = adr64;
> - if (*adr != adr64) {
> - dev_err(dev, "Address out of range\n");
> - return -ERANGE;
> - }
In the conversion, I read it as if we lose this overflow check. Why is that
not a problem?
Cheers,
Peter
> -
> - return 0;
> -}
> -
> -#else
> -
> -static int i2c_mux_gpio_get_acpi_adr(struct device *dev,
> - struct fwnode_handle *fwdev,
> - unsigned int *adr)
> -{
> - return -EINVAL;
> -}
> -
> -#endif
> -
> static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
> struct platform_device *pdev)
> {
> @@ -141,9 +102,9 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
> fwnode_property_read_u32(child, "reg", values + i);
>
> } else if (is_acpi_node(child)) {
> - rc = i2c_mux_gpio_get_acpi_adr(dev, child, values + i);
> + rc = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), values + i);
> if (rc)
> - return rc;
> + return dev_err_probe(dev, rc, "Cannot get address\n");
> }
>
> i++;
>
Hi!
On 2021-11-15 16:42, Andy Shevchenko wrote:
> We have a special helper to get fwnode out of struct device.
> Moreover, dereferencing it directly prevents the fwnode
> modifications in the future.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/i2c/muxes/i2c-mux-gpio.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
> index 31e6eb1591bb..b09c10f36ddb 100644
> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
> @@ -53,6 +53,7 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
> struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> struct device_node *np = dev->of_node;
It feels like there is opportunity to get rid of np, but I suppose that can be
done later...
Acked-by: Peter Rosin <[email protected]>
Cheers,
Peter
> struct device_node *adapter_np;
> struct i2c_adapter *adapter = NULL;
> @@ -60,7 +61,7 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
> unsigned *values;
> int rc, i = 0;
>
> - if (is_of_node(dev->fwnode)) {
> + if (is_of_node(fwnode)) {
> if (!np)
> return -ENODEV;
>
> @@ -72,7 +73,7 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
> adapter = of_find_i2c_adapter_by_node(adapter_np);
> of_node_put(adapter_np);
>
> - } else if (is_acpi_node(dev->fwnode)) {
> + } else if (is_acpi_node(fwnode)) {
> /*
> * In ACPI land the mux should be a direct child of the i2c
> * bus it muxes.
> @@ -111,7 +112,7 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
> }
> mux->data.values = values;
>
> - if (fwnode_property_read_u32(dev->fwnode, "idle-state", &mux->data.idle))
> + if (device_property_read_u32(dev, "idle-state", &mux->data.idle))
> mux->data.idle = I2C_MUX_GPIO_NO_IDLE;
>
> return 0;
>
On 2021-11-15 16:42, Andy Shevchenko wrote:
> Use array_size() helper to aid in 2-factor allocation instances.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Acked-by: Peter Rosin <[email protected]>
Cheers,
Peter
+Cc: Rafael
On Thu, Nov 18, 2021 at 12:24 PM Peter Rosin <[email protected]> wrote:
> On 2021-11-15 16:41, Andy Shevchenko wrote:
...
> > - *adr = adr64;
> > - if (*adr != adr64) {
> > - dev_err(dev, "Address out of range\n");
> > - return -ERANGE;
> > - }
>
> In the conversion, I read it as if we lose this overflow check.
It depends from which angle you look at this. We relaxed requirements.
> Why is that
> not a problem?
The idea behind the acpi_get_local_address() is to provide a unified
way between DT and ACPI for the same value. In either case we take
only a 32-bit value. We might nevertheless add that check to the API.
Rafael, what do you think?
P.S. Just realized that in ACPI the higher part of the address may be
used as flags by some interfaces (SoundWire is one of them), this is
not applicable to I²C muxes right now, but who knows... So I prefer a
relaxed version and, if necessary, documentation should be
amended/updated.
--
With Best Regards,
Andy Shevchenko
On 2021-11-18 11:33, Andy Shevchenko wrote:
> +Cc: Rafael
>
> On Thu, Nov 18, 2021 at 12:24 PM Peter Rosin <[email protected]> wrote:
>> On 2021-11-15 16:41, Andy Shevchenko wrote:
>
> ...
>
>>> - *adr = adr64;
>>> - if (*adr != adr64) {
>>> - dev_err(dev, "Address out of range\n");
>>> - return -ERANGE;
>>> - }
>>
>> In the conversion, I read it as if we lose this overflow check.
>
> It depends from which angle you look at this. We relaxed requirements.
>
>> Why is that
>> not a problem?
>
> The idea behind the acpi_get_local_address() is to provide a unified
> way between DT and ACPI for the same value. In either case we take
> only a 32-bit value. We might nevertheless add that check to the API.
> Rafael, what do you think?
>
> P.S. Just realized that in ACPI the higher part of the address may be
> used as flags by some interfaces (SoundWire is one of them), this is
> not applicable to I²C muxes right now, but who knows... So I prefer a
> relaxed version and, if necessary, documentation should be
> amended/updated.
Splendid, just checking that you're on top of things. I don't think any
doc update is needed on the i2c-mux end, until flags in the upper bits
are introduced? So, looks good to me, thanks!
Acked-by: Peter Rosin <[email protected]>
@Wolfram: You're finding this series in patchwork and will be picking it
up as usual, right? Thanks!
Cheers,
Peter
> @Wolfram: You're finding this series in patchwork and will be picking it
> up as usual, right? Thanks!
Right, will do so now.
On Mon, Nov 15, 2021 at 05:41:59PM +0200, Andy Shevchenko wrote:
> Recently ACPI gained the acpi_get_local_address() API which may be used
> instead of home grown i2c_mux_gpio_get_acpi_adr().
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Applied to for-next, thanks!
On Mon, Nov 15, 2021 at 05:42:00PM +0200, Andy Shevchenko wrote:
> We have a special helper to get fwnode out of struct device.
> Moreover, dereferencing it directly prevents the fwnode
> modifications in the future.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Applied to for-next, thanks!
On Mon, Nov 15, 2021 at 05:42:01PM +0200, Andy Shevchenko wrote:
> Use array_size() helper to aid in 2-factor allocation instances.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Applied to for-next, thanks!