Re: [PATCH RFT 0/2/2] mtd: hyperbus: add Renesas RPC-IF driver

[Vignesh Raghavendra <vigneshr@xxxxxx>]

[...]

>>> So I wonder if it would be a valid option to have a functioning Renesas
>>> Hypeflash driver, first. And in a second step abstract that in a more
>>> generic way to support additional controllers. While in parallel having
>>> a functional driver for the Renesas people, already.
>>
>> AFAICS, the backend driver is not merged and is still in RFC phase.
> 
>    It was still marked RFC back in December and I haven't received any
> feedback since, other than Dirk's request. Where have you been? Well,
> I should have CCed linux-mtd back then... :-/
> 

Well, as you said, this should have been discussed in linux-mtd :-/ And
therefore why my first question on this thread was where is the backend
driver.


>> Therefore I don't see any benefit of two step approach here. Besides
>> you'll have to throw away this new driver (hyperbus/rpc-if.c) entirely
>> later on.
> 
>    Why did you create this directory for, anyway? :-/
> 


This directory is not just for HyperBus controllers but also for flash
drivers.
While HF uses CFI command set, its not necessary that other HyperBus
memory devices do. For example HyperRAM would need a separate driver
which would be under this directory. Even, HF would need a translation
layer from map_* APIs for controllers that can't use map_ APIs directly
and to add HF only features

Then there is need to support HF only controllers such as TI's
HyperBus controller which does not support any of the SPI modes or full
xSPI specification but just initial HyperBus protocol.
I don't think drivers/spi is the right place for such controllers.


>> How difficult is it to rewrite backend to be spi-mem driver? There is
>> already has a spi_mem_ops frontend implementation, so I don't see much
>> of an issue.
> 
>    Really? This may be not much of an issue with coding this but it's
> certainly time consuming (I'm sure there's s/th to think about yet in
> this case)? My management (and also me, so far) believes I'm in the
> final stage with these drivers... what should I say to my boss now?
> 

That's is not my concern and above statements on ML won't help either..
Lets have a constructive discussion and come up with solution is that
maintainable in long term :-/

My aim was to explore whether its possible to support OSPI and HF using
a single driver without needing two frontend drivers and avoid switching
b/w two drivers when supporting xSPI compliant HFs

Again, I did not insist on extending spi_mem_op to be the _only_
option. I said we should have a generic framework to support controllers
such as RPC-IF which supports both OSPI and HF and one of the options is
to extend spi_mem_op.

And I am not responsible for RPC-IF series to go to v19 :(

>> Extending hyperbus core to use spi-mem should also straight forward
>> Would involve moving this patch into core file.
> 
>    Seriously, only "moving"?
> 


HyperBus framework is pretty small and can be refractored quite easily
to support spi_mem_op extension

>>> Is the support for [1] a more or less theoretical one, at the moment? Or
>>> are there users of that which need support "now", too?
>>
>> Its not theoretical, I do see patches for xSPI controller here:
>> https://patchwork.kernel.org/cover/11354193/
> 
>    Which (surprise!) only adds support for the SPI part...
> 
>> So, its best to sort this out now so as to avoid possible backward
>> compatibility issues (especially with DT bindings)
> 
>    What DT issues do you mean exactly? I think that other than changing
> the "home" dir for the bindings, there'd little to change. The "front ends"
> don't deal with the DT probing...
> 

OK, if there are no DT bindings for each of the frontend drivers then
this should not be concern.

Since, Mark seems okay with current approach (per IRC discussions)
and is not keen on extending spi_mem_ops, I will look at this patch as is.

PS: I cannot reply to you on IRC as I am on the opposite end of timezone.

--
Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/