lkml.org 
[lkml]   [2020]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 09/19] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table
On 21/05/20 04:09PM, masonccyang@mxic.com.tw wrote:
>
> Hi Pratyush,
>
> > > > > > + /* Get 8D-8D-8D fast read opcode and dummy cycles. */
> > > > > > + opcode = FIELD_GET(PROFILE1_DWORD1_RD_FAST_CMD, table[0]);
> > > > > > +
> > > > > > + /*
> > > > > > + * Update the fast read settings. We set the default dummy
> > > cycles to
> > > > > 20
> > > > > > + * here. Flashes can change this value if they need to when
> > > enabling
> > > > > > + * octal mode.
> > > > > > + */
> > > > > > +
> spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8_DTR],
> > > > > > + 0, 20, opcode,
> > > > > > + SNOR_PROTO_8_8_8_DTR);
> > > > > > +
> > > > >
> > > > >
> > > > > I thought we have a agreement that only do parse here, no other
> read
> > > > > parameters setting.
> > > >
> > > > Yes, and I considered it. But it didn't make much sense to me to
> > > > introduce an extra member in struct spi_nor just to make this call
> in
> > > > some other function later.
> > > >
> > > > Why exactly do you think doing this here is bad? The way I see it,
> we
> > > > avoid carrying around an extra member in spi_nor and this also
> allows
> > > > flashes to change the read settings easily in a post-sfdp hook. The
> > > > 4bait parsing function does something similar.
> > >
> > > I think it's not a question for good or bad.
> > >
> > > 4bait parsing function parse the 4-Byte Address Instruction Table
> > > and set up read/pp parameters there for sure.
> > >
> > > Here we give the function name spi_nor_parse_profile1() but also
> >
> > But the function that parses 4bait table is also called
> > spi_nor_parse_4bait().
> >
> > > do others setting that has nothing to do with it,
> >
> > Why has setting read opcode and dummy cycles got nothing to do with it?
> > The purpose of the Profile 1.0 table is to tell us the Read Fast
> > command and dummy cycles, among other things. I think it _does_ have
> > something to do with it.
>
> As you know I mean this function just do parse parameter of profile 1
> table
> and keep these value data for later usage.
>
> A device supports xSPI profile table could work in either 8S-8S-8S or
> 8D-8D-8D mode.
> It seems to setup these parameters somewhere out here is betters.

As far as I know, the Profile 1.0 table only describes 8D-8D-8D mode. I
see no mention of 8S-8S-8S in JESD251 or JESD216D.01. No field in the
table describes anything related to 8S. In fact, searching for "8S" in
the JESD251 spec yields 0 results.

Anyway, you should set up 8S parameters in SNOR_CMD_READ_8_8_8, not
SNOR_CMD_READ_8_8_8_DTR. 8D configuration is independent of 8S
configuration.

PS: If you have any more comments, please send them now. The merge
window is getting close, and I'd like to see this make it in.

--
Regards,
Pratyush Yadav
Texas Instruments India

\
 
 \ /
  Last update: 2020-05-21 11:39    [W:1.553 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site