On Tue, Jul 17, 2018 at 9:18 AM Jae Hyun Yoo <jae.hyun....@linux.intel.com> wrote: > > > On 7/15/2018 8:05 PM, Gary Hsu wrote: > > Hi Jae, > > > > In originally, we reserved these register bits for debug purpose. But for > > some error handling case, we found it is also useful to help to clarify > > some error conditions. So driver also can use these fields information to > > check something. > > As for how driver use these information in their code, I have no comment. I > > don’t understand the driver. But these information is the real controller > > state, it had no problem to use information. > > > > Thanks Gary! > > Hi Brendan, > Is it acceptable now if I add this as a comment like below?
Yep, that's fine. I didn't mean to hold you up. I was just curious. > > Thanks, > Jae > > > Best Regards, > > > > 許馥疇 Gary Hsu > > > > 信驊科技股份有限公司 > > ASPEED Technology Inc. > > > > 2F,No.15,Industry East Road 4.,Hsinchu Science Park, Hsinchu City 30077, > > Taiwan > > 新竹科學園區工業東四路 15 號 2F > > > > Tel : 886-3-5789568 ext:807 > > Fax : 886-3-5789586 > > Web : http://www.aspeedtech.com > > > > ************* Email Confidentiality Notice ******************** > > 免責聲明: > > 因應個人資料保護法施行,本信件(或其附件)可能包含機密資訊,並受法律保護。如 台端非指定之收件者,請以電子郵件通知本電子郵件之發送者, > > 並請立即刪除本電子郵件及其附件和銷毀所有複印件。謝謝您的合作! > > > > DISCLAIMER: > > This message (and any attachments) may contain legally privileged and/or > > other confidential information. If you have received it in error, please > > notify the sender by reply e-mail and immediately delete the e-mail and any > > attachments without copying or disclosing the contents. Thank you. > > > > -----Original Message----- > > From: Jae Hyun Yoo [mailto:jae.hyun....@linux.intel.com] > > Sent: Saturday, July 14, 2018 2:54 AM > > To: Brendan Higgins <brendanhigg...@google.com> > > Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>; Joel Stanley > > <j...@jms.id.au>; Andrew Jeffery <and...@aj.id.au>; > > linux-...@vger.kernel.org; OpenBMC Maillist <open...@lists.ozlabs.org>; > > Linux ARM <linux-arm-ker...@lists.infradead.org>; > > linux-asp...@lists.ozlabs.org; Linux Kernel Mailing List > > <linux-kernel@vger.kernel.org>; james.fe...@linux.intel.com; > > vernon.mau...@linux.intel.com; Benjamin Fair <benjaminf...@google.com>; > > Patrick Venture <vent...@google.com>; Gary Hsu <gary_...@aspeedtech.com>; > > Ryan Chen <ryan_c...@aspeedtech.com> > > Subject: Re: [PATCH] i2c: aspeed: Improve driver to support multi-master > > use cases stably > > > > On 7/13/2018 11:12 AM, Brendan Higgins wrote: > >> On Fri, Jul 13, 2018 at 10:22 AM Jae Hyun Yoo > >> <jae.hyun....@linux.intel.com> 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....@linux.intel.com> 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_...@aspeedtech.com> 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 > >>