<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 9 Aug 2019 at 18:26, 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: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 <<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 << NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;<br>
>                 regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, 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></blockquote><div>Indeed our IP limited to 4 address cycle (bytes) do we have NOR Flash with more than 32bit address?</div><div>I will add this limitation thanks!  </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
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></blockquote><div>there is a limitation in a single transfer but I handle it in the <span style="font-weight:bolder;color:rgb(0,0,0);font-family:"Courier New";font-size:11pt">npcm_fiu_manualwrite </span><span style="font-weight:bolder;color:rgb(0,0,0);font-size:11pt"><font face="arial, sans-serif">function</font></span><span style="font-weight:bolder;color:rgb(0,0,0);font-family:"Courier New";font-size:11pt">.</span></div><div><font face="arial, sans-serif"><font color="#000000" style="">Do you suggest to use </font>->adjust_op() instead?</font></div><div><font face="arial, sans-serif"><br></font></div><div><br></div></div></div>