On Fri, Sep 23, 2022 at 06:01:30PM +0800, Binbin Zhou wrote:
> Hi Mika：
>
> 在 2022/9/22 20:26, Mika Westerberg 写道:
> > Hi,
> >
> > On Thu, Sep 22, 2022 at 07:39:55PM +0800, Binbin Zhou wrote:
> > > Add support for the ACPI-based device registration so that the driver
> > > can be also enabled through ACPI table.
> > >
> > > Signed-off-by: Huacai Chen <chenhuacai@xxxxxxxxxxx>
> > > Signed-off-by: Binbin Zhou <zhoubinbin@xxxxxxxxxxx>
> > > ---
> > > drivers/i2c/busses/i2c-gpio.c | 41 ++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 40 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> > > index b1985c1667e1..ccea37e755e6 100644
> > > --- a/drivers/i2c/busses/i2c-gpio.c
> > > +++ b/drivers/i2c/busses/i2c-gpio.c
> > > @@ -13,6 +13,7 @@
> > > #include <linux/init.h>
> > > #include <linux/interrupt.h>
> > > #include <linux/module.h>
> > > +#include <linux/acpi.h>
> > > #include <linux/of.h>
> > > #include <linux/platform_data/i2c-gpio.h>
> > > #include <linux/platform_device.h>
> > > @@ -318,6 +319,24 @@ static void of_i2c_gpio_get_props(struct device_node *np,
> > > of_property_read_bool(np, "i2c-gpio,scl-output-only");
> > > }
> > > +static void acpi_i2c_gpio_get_props(struct device *dev,
> > > + struct i2c_gpio_platform_data *pdata)
> > > +{
> > > + u32 reg;
> > > +
> > > + device_property_read_u32(dev, "delay-us", &pdata->udelay);
> > > +
> > > + if (!device_property_read_u32(dev, "timeout-ms", ®))
> > > + pdata->timeout = msecs_to_jiffies(reg);
> > > +
> > > + pdata->sda_is_open_drain =
> > > + device_property_read_bool(dev, "sda-open-drain");
> > > + pdata->scl_is_open_drain =
> > > + device_property_read_bool(dev, "scl-open-drain");
> > > + pdata->scl_is_output_only =
> > > + device_property_read_bool(dev, "scl-output-only");
> > > +}
> >
> > I think this would work with the DT description too as it is using
> > device_property_xxx() so I wonder if you can just do:
> >
> > i2c_gpio_get_props(dev, pdata);
> >
> > instead of
> >
> > if (np) {
> > of_i2c_gpio_get_props(np, pdata);
> > } else if (ACPI_COMPANION(dev)) {
> > acpi_i2c_gpio_get_props(dev, pdata);
> >
> Sorry, I don't quite understand how to do a unified api.
>
> We get the corresponding value by matching the propname, but obviously the
> propnames related in the two ways are different.
>
> e.g. "delay-us"(ACPI) vs "i2c-gpio, delay-us"(FDT)
Oh, we have different bindings for these? :( That's unfortunate - they
should really have the same. That's the whole purpose of device
properties API in the first place.
