[PATCH v4 0/5] bcm2835: auxiliar device support for spi

[Stephen Warren]

On 08/26/2015 03:28 AM, Martin Sperl wrote:
> 
> On 24.08.2015 10:40, kernel at martin.sperl.org wrote:
>> From: Martin Sperl <kernel at martin.sperl.org>
>>
>> This patch-series implements:
>> * the bcm2835-auxiliar device enable/disable api in soc.
>> * the bcm2835-auxiliar spi device driver
>>
>> The uart1 device driver (ns16550 based) is not implemented so far
>> but should be using the same API.
> 
> As I need to implement another version (see reviews by Stephen)
> here a question related to the uart1 and device-tree (as I now got
> a compute module to test it)
> 
> The uart1 is register-wise compatible with a ns16550, so the
> 8250 driver can get used almost directly.
> 
> Looking at implementations for different platforms I often find:
>     compatible = "xlnx,axi-uart16550-1.01.a",
>              "xlnx,xps-uart16550-2.00.a",
>              "ns16550";
> 
> Also the foundation kernel does something similar.
> 
> The question here is: Would the following device-tree be acceptable?
> uart1: uart at 7e215040 {
>     ...
>     compatible = "bcrm,bcm2835-aux-uart", "ns16550";
>     brcm,aux-enable = <&aux_enable 1>;
>     ...
> };
> 
> So that the driver for "bcrm,bcm2835-aux-uart" would only need to
> contain the "probe" and "release" code to enable/disable the HW block.
> 
> Is this acceptable or would I need to implement a "more" in depth
> driver (so that "ns16550" is not needed in compatiblity) to be accepted
> on the device-tree side?

Sorry for the slow response; I'm travelling at the moment and working
rather long hours.

I believe it's legitimate for a compatible property to include a
particular value (e.g. ns16550) if and only if a driver that knows
solely about that compatible value (i.e. just the 16550 standard
registers) can operate the HW without issue. I don't believe that's true
in this case since the aux enable HW bit must be programmed and does not
exist in standard 16550 HW block.

So, you'd need to do one of:

a) Not put ns16550 in the compatible property. Make a driver that binds
to bcrm,bcm2835-aux-uart. This driver can then hopefully simply
instantiate the standard NS16550 driver to avoid duplicating anything
(e.g. it could create a new child struct {platform_,}device to do this
perhaps).

b) Make the aux node in DT a parent that contains a "standard" ns16550
child node. The parent would fiddle with the aux bits before enumerating
the child nodes (like a bus). The child could then have
compatible="ns16550" just fine, and need no custom driver.