Aspeed 2500 EVB i2c transaction time out

[Shay Slobodkin]

Hi Andrew,

> -----Original Message-----
> From: Andrew Jeffery [mailto:andrew at aj.id.au]
> Sent: Monday, July 25, 2016 4:00 AM
> To: Shay Slobodkin <shays at mellanox.com>; Joel Stanley <joel at jms.id.au>
> Cc: openbmc at lists.ozlabs.org
> Subject: Re: Aspeed 2500 EVB i2c transaction time out
> 
> Hi Shay,
> 
> On Sun, 2016-07-24 at 11:45 +0000, Shay Slobodkin wrote:
> > >
> > > -----Original Message-----
> > > From: joel.stan at gmail.com [mailto:joel.stan at gmail.com] On Behalf Of
> > > Joel Stanley
> > > Sent: Thursday, July 21, 2016 7:36 PM
> > > To: Shay Slobodkin <shays at mellanox.com>
> > > Cc: openbmc at lists.ozlabs.org
> > > Subject: Re: Aspeed 2500 EVB i2c transaction time out
> > Hello Joel,
> >
> > Thanks for your answer.
> > Until we merge from remote the new kernel trees you will push, I was
> > wondering why in aspeed.c for EVB we have the below mapping (there is
> no i2c bus here):
> 
> "no i2c bus here"? if you mean at 1e6e2000 you are right - it's the base
> address of the SCU. In this case it's telling the pinctrl subsystem which pin-
> controller should be used to affect the mux configuration request.
> Otherwise, there are 14 I2C buses, so 3, 4 and 9 are valid.
> 
> > static struct pinctrl_map ast2500_mapping[] __initdata = {
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-8", "1e6e2000.pinmux", NULL,
> > "I2C9"),
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-3", "1e6e2000.pinmux", NULL,
> > "I2C4"),
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-2", "1e6e2000.pinmux", NULL,
> > "I2C3"), };
> >
> > While for Palmetto, we have the below set (with no shift like in EVB:
> > i2c-x - I2C(x+1))
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-3", "1e6e2000.pinmux", NULL,
> > "I2C3"),
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-4", "1e6e2000.pinmux", NULL,
> > "I2C4"),
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-5", "1e6e2000.pinmux", NULL,
> > "I2C5"),
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-6", "1e6e2000.pinmux", NULL,
> > "I2C6"),
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-7", "1e6e2000.pinmux", NULL,
> > "I2C7"),
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-8", "1e6e2000.pinmux", NULL,
> > "I2C8"),
> >         PIN_MAP_MUX_GROUP_DEFAULT("i2c-8", "1e6e2000.pinmux", NULL,
> > "I2C9"),
> >
> > Maybe understanding this could be of some more assistance for us.
> 
> Looks like there is a bug with the palmetto configuration; the I2Cx strings are
> the values from the datasheet and are 1 indexed, while the Linux devices are
> 0 indexed, and so they should be offset.
> 
> It's unfortunate that this made it into the openbmc/linux trees. The patches
> were very much half-baked and we were using them for an internal bring-up
> on an ast2500; we weren't paying enough attention to the ast2400
> configuration and this has slipped through.
> 
> I've mentioned to Joel the idea of backing out the bad pinctrl patches.
> Some were reverted recently, though they removed the GPIO driver hooks
> and not the mux group macros above.
> 
> Cheers,
> 
> Andrew
> 
> PS: if you are interested in testing the most recent pinctrl patches they are
> available at [1]. However, they're based on upstream and so will need a bit of
> massaging to integrate them with the openbmc/linux kernel, for instance
> adding the I2C mux requests to the I2C devicetree nodes.
> 
> [1] https://github.com/amboar/linux/commits/aspeed-pinctrl-4.7-rc7

Thank you. We tried to use this pinctrl, with no luck.
Currently we are still using kernel 4.6 and we receive transaction time out for LM75.
We are pending Joel's 4.7.3 when it is done.

Thanks,
Shay