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

[Martin Sperl <kernel@xxxxxxxxxxxxxxxx>]

> On 22.07.2015, at 03:55, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote:
> 
> However, I'd like to see a "semantic" driver for the shared register
> region rather than a "syscon". IIUC, "syscon" simply provides a stylized
> way for one driver to touch some shared registers directly without any
> semantics. I'd strongly prefer to see a real driver inside Linux rather
> than something that just lets drivers fiddle with the shared registers
> willy nilly. Still, this aspect is an internal implementation detail
> inside the kernel that we can change without external impact later if we
> need.
> 
> More concerning: The bcm283x HW doesn't implement a "syscon" module, but
> some semantic IP block. The DT should contain a real compatible value
> that describes what the HW block really is, not just "syscon". We could
> bind the syscon driver to this compatible value if we have to for

So, do I understand you correctly that if we would call the node 
bcm2835aux_enable as syscon with only the enable bit field register in it 
and add a enable_reg (pointing to the above) and enable_reg_mask=2 or 4
to the spi1/2 nodes then it would be acceptable? 

Would look like this:
spi2@7e2150c0 {
+    compatible = "brcm,bcm2835-aux-spi";
+    reg = <0x7e2150c0 0x40>;
+    interrupts = <1 29>;
+    clocks = <&clk_spi>;
+    #address-cells = <1>;
+    #size-cells = <0>;
+    cs-gpios = <&gpio 43>, <&gpio 44>, <&gpio 45>;
+    enable_reg = <&bcm2835aux_enable>;
+    enable_reg_mask = 4;
+};
+
+/* the necessary syscon config referenced above */
+bcm2835aux_enable: bcm2835aux_enable@0x7e215004 {
+    compatible = "syscon";
+    reg = <0x7e215004 0x04>;
+};


The uart aux driver would use:
+    enable_reg = <&bcm2835aux_enable>;
+    enable_reg_mask = 1;

Or maybe you would prefer a different naming?

The only use-case would be using:
regmap_update_bits(enable_reg, 0, enable_reg_mask, enable_reg_mask); /*enable */
regmap_update_bits(enable_reg, 0, enable_reg_mask, 0); /* disable */


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html