Re: [PATCH] i2c: aspeed: Improve driver to support multi-master use cases stably

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/13/2018 11:12 AM, Brendan Higgins wrote:
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've already CC'ed Gary and Ryan in this thread.

Hi Gary,

Can you explain why the documentation says that the bit field is 'for
debugging purpose only'? Any plan to change the description?

Thanks,

Jae


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




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux