<div dir="ltr">O.K.<div><br></div><div>Thanks a lot for the clarifications</div><div><br></div><div>I will add it in the next patch.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 9 Aug 2019 at 18:51, Boris Brezillon <<a href="mailto:boris.brezillon@collabora.com">boris.brezillon@collabora.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, 9 Aug 2019 18:47:08 +0300<br>
Tomer Maimon <<a href="mailto:tmaimon77@gmail.com" target="_blank">tmaimon77@gmail.com</a>> wrote:<br>
<br>
> On Fri, 9 Aug 2019 at 18:26, Boris Brezillon <<a href="mailto:boris.brezillon@collabora.com" target="_blank">boris.brezillon@collabora.com</a>><br>
> wrote:<br>
> <br>
> > On Fri, 9 Aug 2019 18:26:23 +0300<br>
> > Tomer Maimon <<a href="mailto:tmaimon77@gmail.com" target="_blank">tmaimon77@gmail.com</a>> wrote:<br>
> >  <br>
> > > Hi Boris,<br>
> > ><br>
> > > Thanks a lot for your comment.<br>
> > ><br>
> > > On Thu, 8 Aug 2019 at 18:32, Boris Brezillon <<br>
> > <a href="mailto:boris.brezillon@collabora.com" target="_blank">boris.brezillon@collabora.com</a>><br>
> > > wrote:<br>
> > >  <br>
> > > > On Thu,  8 Aug 2019 16:14:48 +0300<br>
> > > > Tomer Maimon <<a href="mailto:tmaimon77@gmail.com" target="_blank">tmaimon77@gmail.com</a>> wrote:<br>
> > > ><br>
> > > >  <br>
> > > > > +<br>
> > > > > +static const struct spi_controller_mem_ops npcm_fiu_mem_ops = {<br>
> > > > > +     .exec_op = npcm_fiu_exec_op,  <br>
> > > ><br>
> > > > No npcm_supports_op()? That's suspicious, especially after looking at<br>
> > > > the npcm_fiu_exec_op() (and the functions called from there) where the<br>
> > > > requested ->buswidth seems to be completely ignored...<br>
> > > ><br>
> > > > Sorry but I do not fully understand it, do you mean a support for the  <br>
> > > buswidth?<br>
> > > If yes it been done in the UMA functions as follow:<br>
> > ><br>
> > >                 uma_cfg |= ilog2(op->cmd.buswidth);<br>
> > >                 uma_cfg |= ilog2(op->addr.buswidth) <<<br>
> > >                         NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;<br>
> > >                 uma_cfg |= ilog2(op->data.buswidth) <<<br>
> > >                         NPCM_FIU_UMA_CFG_WDBPCK_SHIFT;<br>
> > >                 uma_cfg |= op->addr.nbytes <<  <br>
> > NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;  <br>
> > >                 regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR,  <br>
> > op->addr.val);  <br>
> > >  <br>
> ><br>
> > Hm, the default supports_op() implementation might be just fine for<br>
> > your use case. But there's one thing you still need to check: the<br>
> > number of addr cycles (or address size as you call it in this driver).<br>
> > Looks like your IP is limited to 4 address cycles, if I'm right, you<br>
> > should reject any operation that have op->addr.nbytes > 4. I also<br>
> >  <br>
> Indeed our IP limited to 4 address cycle (bytes) do we have NOR Flash with<br>
> more than 32bit address?<br>
<br>
spi-mem is not only about spi-nor, it can be used for any kind of<br>
memory (NOR, NAND, SRAM, ...) or even to communicate with an FGPA, so<br>
yes, you have to take care of that.<br>
<br>
> I will add this limitation thanks!<br>
> <br>
> > wonder if there's a limitation on the data size you can have on a<br>
> > single transfer. If there's one you should implement ->adjust_op() too.<br>
> >  <br>
> there is a limitation in a single transfer but I handle it in the<br>
> npcm_fiu_manualwrite<br>
> function.<br>
> Do you suggest to use ->adjust_op() instead?<br>
<br>
Yes, should be exposed through ->adjust_op() => the caller needs to<br>
know when a new operation (one containing an opcode+address) is issued,<br>
because sometimes such splits are not supported by the memory.<br>
<br>
</blockquote></div>