Re: [PATCH] i2c: designware: make *CNT values configurable

[Shikhar Dogra (shidogra)]

Hi

On 10/26/17, 7:20 AM, "Jarkko Nikula" <jarkko.nikula@xxxxxxxxxxxxxxx> wrote:

    Hi
    
    On 10/25/2017 09:50 PM, Shikhar Dogra wrote:
    > The values are already configurable from ACPI.
    > 
    > This patch makes the high count (HCNT) and low count (LCNT)
    > register values configurable through device tree.
    > 
    > Cc: xe-linux-external@xxxxxxxxx
    > Signed-off-by: Shikhar Dogra <shidogra@xxxxxxxxx>
    > ---
    >   Documentation/devicetree/bindings/i2c/i2c-designware.txt | 16 ++++++++++++++++
    >   drivers/i2c/busses/i2c-designware-platdrv.c              | 12 ++++++++++++
    >   2 files changed, 28 insertions(+)
    > 
    > diff --git a/Documentation/devicetree/bindings/i2c/i2c-designware.txt b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
    > index fee26dc..2b9ddca 100644
    > --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
    > +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
    > @@ -20,6 +20,18 @@ Optional properties :
    >    - i2c-sda-falling-time-ns : should contain the SDA falling time in nanoseconds.
    >      This value which is by default 300ns is used to compute the tHIGH period.
    >   
    > + - i2c-ss-hcnt : should contain the I2C controller standard speed HCNT value.
    > +   If this is not set we use the calculated and more conservative values.
    > +
    > + - i2c-ss-lcnt : should contain the I2C controller standard speed LCNT value.
    > +   If this is not set we use the calculated and more conservative values.
    > +
    > + - i2c-fs-hcnt : should contain the I2C controller fast speed HCNT value.
    > +   If this is not set we use the calculated and more conservative values.
    > +
    > + - i2c-fs-lcnt : should contain the I2C controller fast speed LCNT value.
    > +   If this is not set we use the calculated and more conservative values.
    > +
    
    Worth to add also properties for high-speed while at it.

Sure, will look at it.

    Out of curiosity, are calculated values by using 
    "i2c-sda-falling-time-ns" and "i2c-scl-falling-time-ns" properties 
    non-optimal on your platform and controller clock?

We did not try with specific values for these in dtb and the default values of 300ns did not work for us.
Platform provided for the optimum *cnt values which we found were configurable via ACPI so, we went ahead to add support for device tree as well.
    
    > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
    > index 6b00061c..25c3b0c 100644
    > --- a/drivers/i2c/busses/i2c-designware-platdrv.c
    > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
    > @@ -177,6 +177,18 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
    >   		of_property_read_u32(pdev->dev.of_node, "clock-frequency",
    >   				     &clk_freq);
    >   
    > +		of_property_read_u16(pdev->dev.of_node,
    > +				     "i2c-ss-hcnt", &dev->ss_hcnt);
    > +
    
    This patch is against old kernel. These were converted near 2 years ago 
    by the commit 4c5301abbf81 ("i2c: designware: Convert to use unified 
    device property API")

Will move the patch to latest kernel (was using v4.4), do the required modification and upload for review again.

Thank you,
Regards
-- 
Shikhar Dogra
SOFTWARE ENGINEER
Core Software Group
shidogra@xxxxxxxxx
Phone: +1 408 527 0543