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

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

2021-03-31 Thread Yicong Yang
Add HiSilicon I2C controller driver for the Kunpeng SoC. It provides the access to the i2c busses, which connects to the eeprom, rtc, etc. The driver works with IRQ mode, and supports basic I2C features and 10bit address. The DMA is not supported. Reviewed-by: Andy Shevchenko Reviewed-by: