Re: [PATCH RFC 2/2] mfd: add Renesas RPC-IF driver

[Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>]

On 06/18/2019 12:57 PM, Lee Jones wrote:

[...]
>>>>>>>> +static int rpcif_probe(struct platform_device *pdev)
>>>>>>>> +{
>>>>>>>> +  struct device_node *flash;
>>>>>>>> +  const struct mfd_cell *cell;
>>>>>>>> +  struct resource *res;
>>>>>>>> +  void __iomem *base;
>>>>>>>> +  struct rpcif *rpc;
>>>>>>>> +
>>>>>>>> +  flash = of_get_next_child(pdev->dev.of_node, NULL);
>>>>>>>> +  if (!flash) {
>>>>>>>> +          dev_warn(&pdev->dev, "no flash node found\n");
>>>>>>>> +          return -ENODEV;
>>>>>>>> +  }
>>>>>>>> +
>>>>>>>> +  if (of_device_is_compatible(flash, "jedec,spi-nor")) {
>>>>>>>> +          cell = &rpcif_spi_ctlr;
>>>>>>>> +  } else if (of_device_is_compatible(flash, "cfi-flash")) {
>>>>>>>> +          cell = &rpcif_hf_ctlr;
>>>>>>>> +  } else {
>>>>>>>> +          dev_warn(&pdev->dev, "unknown flash type\n");
>>>>>>>> +          return -ENODEV;
>>>>>>>> +  }
>>>>>>>
>>>>>>> Why not let DT choose which device to probe?
>>>>>>
>>>>>>    It's the DT that decides here. How else would you imagine that?
>>>>>> It's the same hardware, just the different physical busses that it
>>>>>> commands...
>>>>>
>>>>> DT is not deciding here.  This driver is deciding based on the
>>>>> information contained in the tables - very different thing.
>>>>>
>>>>> Why not just let the OF framework probe the correct device i.e. let it
>>>>> parse the 2 compatible strings and let it match the correct driver for
>>>>> the device?
>>>>
>>>> The OF framework matches against the RPC-IF node, which is a single
>>>> hardware type, hence has a fixed compatible value.
>>>> The mode depends on the subnode in DT, which is something the OF
>>>> framework doesn't match against, so the driver itself has to check the
>>>> subnode's compatible value.
>>>
>>> I can see how it has been implemented.
>>>
>>> It is that which I was questioning.
>>>
>>>> DT describes hardware, not software (Linux subsystem boundary) policy.
>>>
>>> So is an RPC-IF a real hardware device.  Can you share the datasheet?
>>
>> Unfortunately the datasheet for the R-Car Gen3 and RZ/G2 SoCs is
>> not yet public.
> 
> When will it be public?

   Ask Renesas. :-)

> Do you have access to it?

   We do...

> Maybe someone who does can help with the magic number definitions.

   Not very likely. :-)

>> However, a very similar hardware block is present in the RZ/A2M SoC.
>> Please see Chapter 20 ("SPI Multi I/O Bus Controller") of the "RZ/A2M Group
>> User’s Manual: Hardware", which you can download from
>> https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz/rza/rza2m.html#documents

   Also, there's RZ/A1 where a previous version of this hardware seems to be used, see chapter 17
(SPI Multi I/O Bus Controller) of the RZ/A1H/M manual, downloadable from:
https://www.renesas.com/us/en/products/microcontrollers-microprocessors/rz/rza/rza1h.html#documents

>   "The SPI multi I/O bus controller enables the direct connection of
>    serial flash, OctaFlashTM, XccelaTM flash, or HyperFlashTM memory
>    devices to this LSI chip.> 
>    This module allows the connected serial flash, OctaFlashTM, XccelaTM
>    flash, or HyperFlashTM memory devices to be accessed by reading the
>    external address space, or using Manual mode to transmit and receive
>    data."

   For contrast, RZ/A1 didn't yet support OctaFlash and HyperFlash, and R-Car gen3 RPC-IF doesn't
support Xccella yet...

> Looks like a flash device to me.

   More like a multi-protocol flash controller, with the real flash chip connected
to it.

MBR, Sergei