| Message ID | 20180731134740.441-2-alexandre.belloni@bootlin.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for MSCC Ocelot i2c | expand |
On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote: > Switch to device_get_match_data in probe to match the device specific > data > instead of using the acpi specific function. > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c > b/drivers/i2c/busses/i2c-designware-platdrv.c > index ddf13527aaee..00bf62f77c47 100644 > --- a/drivers/i2c/busses/i2c-designware-platdrv.c > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c > @@ -119,10 +119,6 @@ static int dw_i2c_acpi_configure(struct > platform_device *pdev) > break; > } > > - id = acpi_match_device(pdev->dev.driver->acpi_match_table, > &pdev->dev); > - if (id && id->driver_data) > - dev->flags |= (u32)id->driver_data; > - > if (acpi_bus_get_device(handle, &adev)) > return -ENODEV; > > @@ -269,6 +265,8 @@ static int dw_i2c_plat_probe(struct > platform_device *pdev) > else > i2c_parse_fw_timings(&pdev->dev, t, false); > > + dev->flags |= (u32)device_get_match_data(&pdev->dev); > + > acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev); > /* > * Some DSTDs use a non standard speed, round down to the > lowest
On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote: > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote: > > Switch to device_get_match_data in probe to match the device > > specific > > data > > instead of using the acpi specific function. > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Oops. See below. > > - id = acpi_match_device(pdev->dev.driver->acpi_match_table, > > &pdev->dev); > > - if (id && id->driver_data) > > - dev->flags |= (u32)id->driver_data; It seems you forgot to remove unused variable. > > - > > if (acpi_bus_get_device(handle, &adev)) > > return -ENODEV; > > > > @@ -269,6 +265,8 @@ static int dw_i2c_plat_probe(struct > > platform_device *pdev) > > else > > i2c_parse_fw_timings(&pdev->dev, t, false); > > > > + dev->flags |= (u32)device_get_match_data(&pdev->dev); > > + And since it would be changed anyway, can you actually move this line closet to if (has_acpi_companion()) one ? > > acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev); > > /* > > * Some DSTDs use a non standard speed, round down to the > > lowest > >
On 31/07/2018 17:23:04+0300, Andy Shevchenko wrote: > On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote: > > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote: > > > Switch to device_get_match_data in probe to match the device > > > specific > > > data > > > instead of using the acpi specific function. > > > > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Oops. See below. > > > > - id = acpi_match_device(pdev->dev.driver->acpi_match_table, > > > &pdev->dev); > > > - if (id && id->driver_data) > > > - dev->flags |= (u32)id->driver_data; > > It seems you forgot to remove unused variable. > > > > - > > > if (acpi_bus_get_device(handle, &adev)) > > > return -ENODEV; > > > > > > @@ -269,6 +265,8 @@ static int dw_i2c_plat_probe(struct > > > platform_device *pdev) > > > else > > > i2c_parse_fw_timings(&pdev->dev, t, false); > > > > > > + dev->flags |= (u32)device_get_match_data(&pdev->dev); > > > + > > And since it would be changed anyway, can you actually move this line > closet to if (has_acpi_companion()) one ? > I need that value to be set before calling of_configure though. > > > acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev); > > > /* > > > * Some DSTDs use a non standard speed, round down to the > > > lowest > > > > > > -- > Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Intel Finland Oy
On Tue, 2018-07-31 at 16:30 +0200, Alexandre Belloni wrote: > On 31/07/2018 17:23:04+0300, Andy Shevchenko wrote: > > On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote: > > > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote: > > > > + dev->flags |= (u32)device_get_match_data(&pdev->dev); > > > > + > > > > And since it would be changed anyway, can you actually move this > > line > > closet to if (has_acpi_companion()) one ? > > > > I need that value to be set before calling of_configure though. AFAICS, you added those lines later, so, it would be something like dev->flags |= ... if (of_node) of_configure() if (has_acpi_companion()) acpi_configure() Would it work for you?
On 31/07/2018 17:53:17+0300, Andy Shevchenko wrote: > On Tue, 2018-07-31 at 16:30 +0200, Alexandre Belloni wrote: > > On 31/07/2018 17:23:04+0300, Andy Shevchenko wrote: > > > On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote: > > > > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote: > > > > > > + dev->flags |= (u32)device_get_match_data(&pdev->dev); > > > > > + > > > > > > And since it would be changed anyway, can you actually move this > > > line > > > closet to if (has_acpi_companion()) one ? > > > > > > > I need that value to be set before calling of_configure though. > > AFAICS, you added those lines later, so, it would be something like > > dev->flags |= ... > > if (of_node) > of_configure() > if (has_acpi_companion()) > acpi_configure() > > Would it work for you? > Works for me, I'll send v3 soon.
On 31/07/2018 17:23:04+0300, Andy Shevchenko wrote: > On Tue, 2018-07-31 at 17:02 +0300, Andy Shevchenko wrote: > > On Tue, 2018-07-31 at 15:47 +0200, Alexandre Belloni wrote: > > > Switch to device_get_match_data in probe to match the device > > > specific > > > data > > > instead of using the acpi specific function. > > > > > > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Oops. See below. > > > > - id = acpi_match_device(pdev->dev.driver->acpi_match_table, > > > &pdev->dev); > > > - if (id && id->driver_data) > > > - dev->flags |= (u32)id->driver_data; > I'll change that cast to (uintptr_t). Else gcc complains about the size mismatch on 64 bit platform. I'm letting kbuild play with my branch a bit before sending v3.
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index ddf13527aaee..00bf62f77c47 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -119,10 +119,6 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev) break; } - id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev); - if (id && id->driver_data) - dev->flags |= (u32)id->driver_data; - if (acpi_bus_get_device(handle, &adev)) return -ENODEV; @@ -269,6 +265,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) else i2c_parse_fw_timings(&pdev->dev, t, false); + dev->flags |= (u32)device_get_match_data(&pdev->dev); + acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev); /* * Some DSTDs use a non standard speed, round down to the lowest
Switch to device_get_match_data in probe to match the device specific data instead of using the acpi specific function. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> --- drivers/i2c/busses/i2c-designware-platdrv.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)