On Fri, Jul 13, 2018 at 10:22 AM Jae Hyun Yoo <jae.hyun.yoo@xxxxxxxxxxxxxxx> wrote: > > On 7/12/2018 11:21 AM, Jae Hyun Yoo wrote: > > On 7/12/2018 2:33 AM, Brendan Higgins wrote: > >> On Wed, Jun 27, 2018 at 10:55 AM Jae Hyun Yoo > >> <jae.hyun.yoo@xxxxxxxxxxxxxxx> wrote: <snip> > >> <snip> > >>>>> + for (;;) { > >>>>> + if (!(readl(bus->base + ASPEED_I2C_CMD_REG) & > >>>>> + (ASPEED_I2CD_BUS_BUSY_STS | > >>>>> + ASPEED_I2CD_XFER_MODE_STS_MASK))) > >>>> > >>>> Is using the Transfer Mode State Machine bits necessary? The > >>>> documentation marks it as "for debugging purpose only," so relying on > >>>> it makes me nervous. > >>>> > >>> > >>> As you said, the documentation marks it as "for debugging purpose only." > >>> but ASPEED also uses this way in their SDK code because it's the best > >>> way for checking bus busy status which can cover both single and > >>> multi-master use cases. > >>> > >> > >> Well, it would also be really nice to have access to this bit if > >> someone wants > >> to implement MCTP. Could we maybe check with Aspeed what them meant by > >> "for > >> debugging purposes only" and document it here? It makes me nervous to > >> rely on > >> debugging functionality for normal usage. > >> > > > > Okay, I'll check it with Aspeed. Will let you know their response. > > > > I've checked it with Gary Hsu <gary_hsu@xxxxxxxxxxxxxx> and he confirmed > that the bits reflect real information and good to be used in practical > code. Huh. For my own edification, could you ask them why they said "for debugging purpose only" in the documentation? I am just really curious what they meant by that. I would be satisfied if you just CC'ed me on your email thread with Gary, and I can ask him myself. > > I'll add a comment like below: > > /* > * This is marked as 'for debugging purpose only' in datasheet but > * ASPEED confirmed that this reflects real information and good > * to be used in practical code. > */ > > Is it acceptable then? Yeah, that's fine. <snip> Cheers