| Message ID | 20170630005408.23968-6-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 30, 2017 at 3:54 AM, Chris Packham <chris.packham@alliedtelesis.co.nz> wrote: > Use device_property_read_u32 instead of of_property_read_u32_index to > lookup the "clock-frequency" property. My comments below. > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > drivers/i2c/busses/i2c-pca-platform.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c > index 1e3c247de8f8..80420f753a87 100644 > --- a/drivers/i2c/busses/i2c-pca-platform.c > +++ b/drivers/i2c/busses/i2c-pca-platform.c > @@ -176,13 +176,12 @@ static int i2c_pca_pf_probe(struct platform_device *pdev) > if (platform_data) { > i2c->adap.timeout = platform_data->timeout; > i2c->algo_data.i2c_clock = platform_data->i2c_clock_speed; > } else { > i2c->adap.timeout = HZ; > - i2c->algo_data.i2c_clock = 59000; > + ret = device_property_read_u32(&pdev->dev, "clock-frequency", > + &i2c->algo_data.i2c_clock); > + if (ret) > + i2c->algo_data.i2c_clock = 59000; My idea is to get rid of legacy platform data completely. That's why I suggested device_* in the first place. In similar way like you did with GPIO lookup table, you may use PROPERTY_ENTRY*() macros in the board files. Does it make sense?
> > - i2c->algo_data.i2c_clock = 59000; > > + ret = device_property_read_u32(&pdev->dev, "clock-frequency", > > + &i2c->algo_data.i2c_clock); > > + if (ret) > > + i2c->algo_data.i2c_clock = 59000; > > My idea is to get rid of legacy platform data completely. > That's why I suggested device_* in the first place. > > In similar way like you did with GPIO lookup table, you may use > PROPERTY_ENTRY*() macros in the board files. > > Does it make sense? Frankly, I am not a big fan of converting board files if we cannot test the changes.
On Fri, Jun 30, 2017 at 12:03 PM, Wolfram Sang <wsa@the-dreams.de> wrote: > >> > - i2c->algo_data.i2c_clock = 59000; >> > + ret = device_property_read_u32(&pdev->dev, "clock-frequency", >> > + &i2c->algo_data.i2c_clock); >> > + if (ret) >> > + i2c->algo_data.i2c_clock = 59000; >> >> My idea is to get rid of legacy platform data completely. >> That's why I suggested device_* in the first place. >> >> In similar way like you did with GPIO lookup table, you may use >> PROPERTY_ENTRY*() macros in the board files. >> >> Does it make sense? > > Frankly, I am not a big fan of converting board files if we cannot test > the changes. So, if no one is using that old boards, should we really take care more than just compile test? P.S. Legacy platform data makes a burden of development nowadays. Built-in device properties API (as a part of Unified Device Properties) is exactly for getting rid of legacy stuff and make things much cleaner.
On 30/06/17 22:56, Andy Shevchenko wrote: > On Fri, Jun 30, 2017 at 12:03 PM, Wolfram Sang <wsa@the-dreams.de> wrote: >> >>>> - i2c->algo_data.i2c_clock = 59000; >>>> + ret = device_property_read_u32(&pdev->dev, "clock-frequency", >>>> + &i2c->algo_data.i2c_clock); >>>> + if (ret) >>>> + i2c->algo_data.i2c_clock = 59000; >>> >>> My idea is to get rid of legacy platform data completely. >>> That's why I suggested device_* in the first place. >>> >>> In similar way like you did with GPIO lookup table, you may use >>> PROPERTY_ENTRY*() macros in the board files. >>> >>> Does it make sense? >> >> Frankly, I am not a big fan of converting board files if we cannot test >> the changes. > > So, if no one is using that old boards, should we really take care > more than just compile test? > > P.S. Legacy platform data makes a burden of development nowadays. > Built-in device properties API (as a part of Unified Device > Properties) is exactly for getting rid of legacy stuff and make things > much cleaner. We could probably go with an approach of making the device properties the default which would suit the new style and leave the platform_data to override things if it is present. If/when the older platforms go away we can drop struct i2c_pca9564_pf_platform_data. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> Frankly, I am not a big fan of converting board files if we cannot test > >> the changes. > > > > So, if no one is using that old boards, should we really take care > > more than just compile test? > > > > P.S. Legacy platform data makes a burden of development nowadays. > > Built-in device properties API (as a part of Unified Device > > Properties) is exactly for getting rid of legacy stuff and make things > > much cleaner. > > We could probably go with an approach of making the device properties > the default which would suit the new style and leave the platform_data > to override things if it is present. > > If/when the older platforms go away we can drop struct > i2c_pca9564_pf_platform_data. I don't have a super strong opinion, so we can discuss this for 4.14. For 4.13, I picked the two easy patches already. Thanks!
diff --git a/drivers/i2c/busses/i2c-pca-platform.c b/drivers/i2c/busses/i2c-pca-platform.c index 1e3c247de8f8..80420f753a87 100644 --- a/drivers/i2c/busses/i2c-pca-platform.c +++ b/drivers/i2c/busses/i2c-pca-platform.c @@ -176,13 +176,12 @@ static int i2c_pca_pf_probe(struct platform_device *pdev) if (platform_data) { i2c->adap.timeout = platform_data->timeout; i2c->algo_data.i2c_clock = platform_data->i2c_clock_speed; - } else if (np) { - i2c->adap.timeout = HZ; - of_property_read_u32_index(np, "clock-frequency", 0, - &i2c->algo_data.i2c_clock); } else { i2c->adap.timeout = HZ; - i2c->algo_data.i2c_clock = 59000; + ret = device_property_read_u32(&pdev->dev, "clock-frequency", + &i2c->algo_data.i2c_clock); + if (ret) + i2c->algo_data.i2c_clock = 59000; } i2c->gpio = devm_gpiod_get_optional(&pdev->dev, "reset-gpios", GPIOD_OUT_LOW);
Use device_property_read_u32 instead of of_property_read_u32_index to lookup the "clock-frequency" property. Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- drivers/i2c/busses/i2c-pca-platform.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)