[PATCH 1/2] spi: bcm2835: add spi-bcm2835aux driver for the auxiliar spi1 and spi2

[Martin Sperl]

> On 24.07.2015, at 06:09, Stephen Warren <swarren at wwwdotorg.org> wrote:
> 
> I think I'd expect the shared registers to be:
> 
> bcm2835aux: misc at 0x7e215000 {
>    compatible = "brcm,bcm2835-aux";
>    reg = <0x7e215000 0x08>;
> };
> 
> There are two 4-byte registers outside the UART/SPI blocks, and the
> compatible value should reflect what the block is, not that Linux may
> use a syscon driver to implement the driver for it.
> 
> In the client, I'd expect a more semantic naming for the reference;
> something like:
> 
> 	brcm,aux = <&bcm2835aux 4>;
> 
> brcm, since it's a custom/vendor-specific property. aux is the name of
> the object that's referenced. Packing the phandle and data together into
> a single property reduces the number of separate properties, and is a
> typical thing to do in a client of a service in DT.

So in the end you are saying we need a separate driver to get written
(because of ‘compatible = "brcm,bcm2835-aux”;’)

That is unless you would allow:
compatible = compatible = "brcm,bcm2835-aux”, “syscon”;

If this is not acceptable, then where should such a driver go in the
kernel tree?

It would essentially implement the following:
bcm2835aux_enable(dev, device-id);
bcm2835aux_disable(dev, device-id);

Which would just set/clean the bits in the register while holding a lock.

As an alternative: maybe we could implement it as an IRQ driver
and when the irq is requested for that device, then the HW-block gets
enabled automatically (and disabled when released).

That way we may not need to have a separate driver that would enable
the uart1, but it would be sufficient to configure it as follows:

uart1: uart at 7e215040 {
	compatible = "brcm,bcm2835-aux-uart", "ns16550";
	reg = <0x7e215040 0x40>;
	interrupts = <X 0>;
	clock-frequency = <500000000>;
	reg-shift = <2>;
	no-loopback-test;
	status = "disabled";
};

(taken from the foundation device-tree)

Please clarify

Thanks,
Martin