Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1721778 > unrolled thread

Re: [PATCH] i2c: aspeed: Retain delay/setup/hold values when configuring bus frequency

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] i2c: aspeed: Retain delay/setup/hold values when  configuring bus frequency Wolfram Sang <wsa@the-dreams.de> - 2017-08-28 18:10 +0200
    Re: [PATCH] i2c: aspeed: Retain delay/setup/hold values when  configuring bus frequency Andrew Jeffery <andrew@aj.id.au> - 2017-08-29 09:00 +0200
      Re: [PATCH] i2c: aspeed: Retain delay/setup/hold values when  configuring bus frequency Wolfram Sang <wsa@the-dreams.de> - 2017-08-29 10:50 +0200
        Re: [PATCH] i2c: aspeed: Retain delay/setup/hold values when  configuring bus frequency Andrew Jeffery <andrew@aj.id.au> - 2017-08-29 10:50 +0200

#1721778 — Re: [PATCH] i2c: aspeed: Retain delay/setup/hold values when configuring bus frequency

[Multipart message — attachments visible in raw view] — view raw

On Tue, Aug 15, 2017 at 04:51:02PM +0930, Andrew Jeffery wrote:
> In addition to the base, low and high clock configuration, the AC timing
> register #1 on the AST2400 houses fields controlling:
> 
> 1. tBUF: Minimum delay between Stop and Start conditions
> 2. tHDSTA: Hold time for the Start condition
> 3. tACST: Setup time for Start and Stop conditions, and hold time for the
>    Repeated Start condition
> 
> These values are defined in hardware on the AST2500 and therefore don't
> need to be set.
> 
> aspeed_i2c_init_clk() was performing a direct write of the generated
> clock values rather than a read/mask/modify/update sequence to retain
> tBUF, tHDSTA and tACST, and therefore cleared the tBUF, tHDSTA and tACST
> fields on the AST2400. This resulted in a delay/setup/hold time of 1
> base clock, which in some configurations is not enough for some devices
> (e.g. the MAX31785 fan controller, with an APB of 48MHz and a desired
> bus speed of 100kHz).
> 
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

Applied to for-next, thanks! I even considered for-current but it does
not apply there. So, I leave the backporting for the interested parties
:)

[toc] | [next] | [standalone]

#1722187

[Multipart message — attachments visible in raw view] — view raw

On Mon, 2017-08-28 at 18:07 +0200, Wolfram Sang wrote:
> On Tue, Aug 15, 2017 at 04:51:02PM +0930, Andrew Jeffery wrote:
> > In addition to the base, low and high clock configuration, the AC timing
> > register #1 on the AST2400 houses fields controlling:
> > 
> > 1. tBUF: Minimum delay between Stop and Start conditions
> > 2. tHDSTA: Hold time for the Start condition
> > 3. tACST: Setup time for Start and Stop conditions, and hold time for the
> >    Repeated Start condition
> > 
> > These values are defined in hardware on the AST2500 and therefore don't
> > need to be set.
> > 
> > aspeed_i2c_init_clk() was performing a direct write of the generated
> > clock values rather than a read/mask/modify/update sequence to retain
> > tBUF, tHDSTA and tACST, and therefore cleared the tBUF, tHDSTA and tACST
> > fields on the AST2400. This resulted in a delay/setup/hold time of 1
> > base clock, which in some configurations is not enough for some devices
> > (e.g. the MAX31785 fan controller, with an APB of 48MHz and a desired
> > bus speed of 100kHz).
> > 
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> 
> Applied to for-next, thanks! 

Thanks!

> I even considered for-current but it does
> not apply there. So, I leave the backporting for the interested parties
> :)
> 

It depends on Brendan's clock divisor calculation fix, which appears to
be in for-next but not for-current:

    87b59ff8d1d9 i2c: aspeed: add proper support fo 24xx clock params

I'd argue that Brendan's patch should go in for-current as well,
because it fixes a divisor rounding error for the ast2500 (bus is
clocked faster than requested).

Cheers,

Andrew

[toc] | [prev] | [next] | [standalone]

#1722234

[Multipart message — attachments visible in raw view] — view raw

> I'd argue that Brendan's patch should go in for-current as well,
> because it fixes a divisor rounding error for the ast2500 (bus is
> clocked faster than requested).

Hmmm, pity, the description said "potential" issue so I decided for
for-next. However, I wouldn't like to reshuffle my branches much so
short before the merge window. So, would it be OK with you that you send
both patches to stable after rc1?

[toc] | [prev] | [next] | [standalone]

#1722236

[Multipart message — attachments visible in raw view] — view raw

On Tue, 2017-08-29 at 10:45 +0200, Wolfram Sang wrote:
> > I'd argue that Brendan's patch should go in for-current as well,
> > because it fixes a divisor rounding error for the ast2500 (bus is
> > clocked faster than requested).
> 
> Hmmm, pity, the description said "potential" issue so I decided for
> for-next. However, I wouldn't like to reshuffle my branches much so
> short before the merge window. So, would it be OK with you that you send
> both patches to stable after rc1?
> 

Will do.

Cheers,

Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web