Re: [PATCH v6 3/5] i2c: add support for HiSilicon I2C controller

2021-04-08 Thread Andy Shevchenko
On Thu, Apr 8, 2021 at 10:17 AM Yicong Yang wrote: > On 2021/4/8 7:04, Wolfram Sang wrote: > > > >> Reason for temp variable is for me it's confusing to see statement like > >> "rate_khz = rate_khz / 1000". > > > > Yes. And with this clearer calculation, we can maybe skip the HZ_PER_KHZ > >

Re: [PATCH v6 3/5] i2c: add support for HiSilicon I2C controller

2021-04-08 Thread Yicong Yang
On 2021/4/8 7:04, Wolfram Sang wrote: > >> Reason for temp variable is for me it's confusing to see statement like >> "rate_khz = rate_khz / 1000". > > Yes. And with this clearer calculation, we can maybe skip the HZ_PER_KHZ > define completely and just use plain '1000' as a factor/divider

Re: [PATCH v6 3/5] i2c: add support for HiSilicon I2C controller

2021-04-07 Thread Wolfram Sang
> Reason for temp variable is for me it's confusing to see statement like > "rate_khz = rate_khz / 1000". Yes. And with this clearer calculation, we can maybe skip the HZ_PER_KHZ define completely and just use plain '1000' as a factor/divider because it then becomes obvious. I still find the

Re: [PATCH v6 3/5] i2c: add support for HiSilicon I2C controller

2021-04-07 Thread Yicong Yang
On 2021/4/7 16:32, Jarkko Nikula wrote: > Hi > > On 3/31/21 4:36 PM, Yicong Yang wrote: >> +    ret = device_property_read_u64(dev, "clk_rate", >clk_rate_khz); >> +    if (ret) { >> +    dev_err(dev, "failed to get clock frequency, ret = %d\n", ret); >> +    return ret; >> +    } >> + >>

Re: [PATCH v6 3/5] i2c: add support for HiSilicon I2C controller

2021-04-07 Thread Jarkko Nikula
Hi On 3/31/21 4:36 PM, Yicong Yang wrote: + ret = device_property_read_u64(dev, "clk_rate", >clk_rate_khz); + if (ret) { + dev_err(dev, "failed to get clock frequency, ret = %d\n", ret); + return ret; + } + + ctlr->clk_rate_khz =

Re: [PATCH v6 3/5] i2c: add support for HiSilicon I2C controller

2021-04-07 Thread Yicong Yang
On 2021/4/7 4:02, Wolfram Sang wrote: > > Only super minor stuff. Thanks to all the contributors and reviewers! > >> +#define HZ_PER_KHZ 1000 > > KHZ_PER_HZ? that doesn't match what we want. we want the count of HZs per one KHZ. > >> +ret = devm_i2c_add_adapter(dev, adapter); >> +if

Re: [PATCH v6 3/5] i2c: add support for HiSilicon I2C controller

2021-04-06 Thread Wolfram Sang
Only super minor stuff. Thanks to all the contributors and reviewers! > +#define HZ_PER_KHZ 1000 KHZ_PER_HZ? > + ret = devm_i2c_add_adapter(dev, adapter); > + if (ret) { > + dev_err(dev, "failed to add i2c adapter, ret = %d\n", ret); No need to print that. The core