Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1281137 > unrolled thread

[PATCH 1/3] Device tree binding documentation for chromeos-firmware

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/3] Device tree binding documentation for chromeos-firmware Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-01 20:20 +0100
    Re: [PATCH 1/3] Device tree binding documentation for  chromeos-firmware Rob Herring <robh@kernel.org> - 2015-12-02 16:20 +0100
      Re: [PATCH 1/3] Device tree binding documentation for chromeos-firmware Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-02 18:00 +0100
        Re: [PATCH 1/3] Device tree binding documentation for chromeos-firmware Rob Herring <robh@kernel.org> - 2015-12-02 19:50 +0100
          Re: [PATCH 1/3] Device tree binding documentation for chromeos-firmware Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-02 22:50 +0100
      Re: [PATCH 1/3] Device tree binding documentation for chromeos-firmware Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-03 11:20 +0100
        Re: [PATCH 1/3] Device tree binding documentation for chromeos-firmware Rob Herring <robh@kernel.org> - 2015-12-03 16:10 +0100
          Re: [PATCH 1/3] Device tree binding documentation for chromeos-firmware Martyn Welch <martyn.welch@collabora.co.uk> - 2015-12-03 17:20 +0100

#1281137 — [PATCH 1/3] Device tree binding documentation for chromeos-firmware

This patch adds documentation for the chromeos-firmware binding.

Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Cc: devicetree@vger.kernel.org
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
 .../devicetree/bindings/misc/chromeos-firmware.txt | 27 ++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/chromeos-firmware.txt

diff --git a/Documentation/devicetree/bindings/misc/chromeos-firmware.txt b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
new file mode 100644
index 0000000..8240611
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
@@ -0,0 +1,27 @@
+Device-Tree bindings for chromeos-firmware.c.
+
+Required properties:
+	- compatible = "google,gpio-firmware";
+
+Each signal is represented as a sub-node of "chromeos_firmware":
+Subnode properties:
+
+	- gpios: OF device-tree gpio specification.
+
+Example nodes:
+
+	chromeos_firmware {
+		compatible = "google,gpio-firmware";
+
+		write-protect {
+			gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
+		};
+
+		developer-switch {
+			gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
+		};
+
+		recovery-switch {
+			gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
+		};
+	};
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1281898 — Re: [PATCH 1/3] Device tree binding documentation for chromeos-firmware

On Tue, Dec 01, 2015 at 07:12:49PM +0000, Martyn Welch wrote:
> This patch adds documentation for the chromeos-firmware binding.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> ---
>  .../devicetree/bindings/misc/chromeos-firmware.txt | 27 ++++++++++++++++++++++

bindings/firmware/ please.

>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/chromeos-firmware.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/chromeos-firmware.txt b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
> new file mode 100644
> index 0000000..8240611
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
> @@ -0,0 +1,27 @@
> +Device-Tree bindings for chromeos-firmware.c.

Perhaps a bit more description what this is.

What aspect of this is firmware? How does this relate to the EC?

> +
> +Required properties:
> +	- compatible = "google,gpio-firmware";

No versions?

> +
> +Each signal is represented as a sub-node of "chromeos_firmware":
> +Subnode properties:
> +
> +	- gpios: OF device-tree gpio specification.
> +
> +Example nodes:
> +
> +	chromeos_firmware {

This should go under /firmware

> +		compatible = "google,gpio-firmware";
> +
> +		write-protect {

You need to define what are valid sub nodes. The example is not 
documentation.

> +			gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		developer-switch {
> +			gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		recovery-switch {
> +			gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1282017

On 02/12/15 15:15, Rob Herring wrote:
> On Tue, Dec 01, 2015 at 07:12:49PM +0000, Martyn Welch wrote:
>> This patch adds documentation for the chromeos-firmware binding.
>>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Pawel Moll <pawel.moll@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>> Cc: Kumar Gala <galak@codeaurora.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>> ---
>>   .../devicetree/bindings/misc/chromeos-firmware.txt | 27 ++++++++++++++++++++++
>
> bindings/firmware/ please.
>

OK.

>>   1 file changed, 27 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>
>> diff --git a/Documentation/devicetree/bindings/misc/chromeos-firmware.txt b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>> new file mode 100644
>> index 0000000..8240611
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>> @@ -0,0 +1,27 @@
>> +Device-Tree bindings for chromeos-firmware.c.
>
> Perhaps a bit more description what this is.
>
> What aspect of this is firmware? How does this relate to the EC?
>

With respect to write-protect, this line is the write protection for the 
flash which holds the bootloader.

For the developer-switch and recovery-switch, I understand that pulling 
these lines low result in the stock firmware forcing the device to boot 
into developer mode and recovery mode respectively. The device I have no 
longer runs the stock firmware, so I'm not able to confirm this, though 
I am able to drive these lines.

As far as I'm aware, none of these are related to the operation of the EC.

Will update the binding documentation.

>> +
>> +Required properties:
>> +	- compatible = "google,gpio-firmware";
>
> No versions?
>

I'm not aware of any and would rather not start inventing ones that 
aren't already there.

>> +
>> +Each signal is represented as a sub-node of "chromeos_firmware":
>> +Subnode properties:
>> +
>> +	- gpios: OF device-tree gpio specification.
>> +
>> +Example nodes:
>> +
>> +	chromeos_firmware {
>
> This should go under /firmware

Ok, will do.

>
>> +		compatible = "google,gpio-firmware";
>> +
>> +		write-protect {
>
> You need to define what are valid sub nodes. The example is not
> documentation.
>

Ok

>> +			gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
>> +		};
>> +
>> +		developer-switch {
>> +			gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
>> +		};
>> +
>> +		recovery-switch {
>> +			gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
>> +		};
>> +	};
>> --
>> 2.1.4
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1282265

On Wed, Dec 2, 2015 at 10:49 AM, Martyn Welch
<martyn.welch@collabora.co.uk> wrote:
>
>
> On 02/12/15 15:15, Rob Herring wrote:
>>
>> On Tue, Dec 01, 2015 at 07:12:49PM +0000, Martyn Welch wrote:
>>>
>>> This patch adds documentation for the chromeos-firmware binding.
>>>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>>> Cc: Kumar Gala <galak@codeaurora.org>
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>>> ---
>>>   .../devicetree/bindings/misc/chromeos-firmware.txt | 27
>>> ++++++++++++++++++++++
>>
>>
>> bindings/firmware/ please.
>>
>
> OK.
>
>>>   1 file changed, 27 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>> b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>> new file mode 100644
>>> index 0000000..8240611
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>> @@ -0,0 +1,27 @@
>>> +Device-Tree bindings for chromeos-firmware.c.
>>
>>
>> Perhaps a bit more description what this is.
>>
>> What aspect of this is firmware? How does this relate to the EC?
>>
>
> With respect to write-protect, this line is the write protection for the
> flash which holds the bootloader.

What is driving the write-protect? Are trying to assign ownership of
the SOC GPIOs to the bootloader/firmware? If so, I think this is all
wrong.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1282450

On 02/12/15 18:44, Rob Herring wrote:
> On Wed, Dec 2, 2015 at 10:49 AM, Martyn Welch
> <martyn.welch@collabora.co.uk> wrote:
>>
>>
>> On 02/12/15 15:15, Rob Herring wrote:
>>>
>>> On Tue, Dec 01, 2015 at 07:12:49PM +0000, Martyn Welch wrote:
>>>>
>>>> This patch adds documentation for the chromeos-firmware binding.
>>>>
>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>>>> Cc: Kumar Gala <galak@codeaurora.org>
>>>> Cc: devicetree@vger.kernel.org
>>>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>>>> ---
>>>>    .../devicetree/bindings/misc/chromeos-firmware.txt | 27
>>>> ++++++++++++++++++++++
>>>
>>>
>>> bindings/firmware/ please.
>>>
>>
>> OK.
>>
>>>>    1 file changed, 27 insertions(+)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>> b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>> new file mode 100644
>>>> index 0000000..8240611
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>> @@ -0,0 +1,27 @@
>>>> +Device-Tree bindings for chromeos-firmware.c.
>>>
>>>
>>> Perhaps a bit more description what this is.
>>>
>>> What aspect of this is firmware? How does this relate to the EC?
>>>
>>
>> With respect to write-protect, this line is the write protection for the
>> flash which holds the bootloader.
>
> What is driving the write-protect? Are trying to assign ownership of
> the SOC GPIOs to the bootloader/firmware? If so, I think this is all
> wrong.
>

The lines are typically driven by a debugging board plugged into a 
socket on the Chromebooks motherboard, not by the device it's self. The 
driver exposes a read-only interface to these signals.

Martyn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1282900

On 02/12/15 15:15, Rob Herring wrote:
> On Tue, Dec 01, 2015 at 07:12:49PM +0000, Martyn Welch wrote:
>> This patch adds documentation for the chromeos-firmware binding.
>>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Pawel Moll <pawel.moll@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>> Cc: Kumar Gala <galak@codeaurora.org>
>> Cc: devicetree@vger.kernel.org
>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>> ---
>>   .../devicetree/bindings/misc/chromeos-firmware.txt | 27 ++++++++++++++++++++++
>
> bindings/firmware/ please.
>
>>   1 file changed, 27 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>
>> diff --git a/Documentation/devicetree/bindings/misc/chromeos-firmware.txt b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>> new file mode 100644
>> index 0000000..8240611
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>> @@ -0,0 +1,27 @@

<snip>

>> +
>> +Each signal is represented as a sub-node of "chromeos_firmware":
>> +Subnode properties:
>> +
>> +	- gpios: OF device-tree gpio specification.
>> +
>> +Example nodes:
>> +
>> +	chromeos_firmware {
>
> This should go under /firmware
>

I've changed this to be:

	firmware {
		chromeos {
			...
		};
	];

Which I generally accept (assuming this is considered a part of the 
firmware) as a better way to represent this in the device tree, however 
this has the nasty side effect of causing the device tree parsing to 
avoid parsing the chromeos child and seeing it's compatible property (as 
the firmware node isn't a bus), resulting in the probe routine not being 
called.

If I add a 'compatible = "simple-bus"' property to the firmware node it 
works, but this doesn't seem quite right as I believe "simple-bus" is 
defined as a "simple memory mapped bus".

I /could/ rewrite the initialisation to call of_find_compatible_node(), 
but this seems rather hacky and inefficient. I can think of 2 other ways 
this could be resolved:

(1) As this is only tangentially related to firmware, I rename it 
something like "chromeos-signals" and make it it's own node. In essence 
this driver provides a mechanism built on top of specific GPIO (ala 
gpio-keys seems to be, after-all this has a similar use of resources to 
that).

(2) Add a compatible string something like 'compatible="logical-group";' 
to the firmware node and add that too the bus matching logic. This would 
have the advantage of solving this in the general case (I guess there 
are other instances where a grouping of things more logically rather 
than physically connected would ideally be grouped together), though I 
expect there may be some strong views regarding this approach.

Would either of those be acceptable or is there a better way of 
resolving this that I've missed?

Martyn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283107

On Thu, Dec 3, 2015 at 4:14 AM, Martyn Welch
<martyn.welch@collabora.co.uk> wrote:
>
> On 02/12/15 15:15, Rob Herring wrote:
>>
>> On Tue, Dec 01, 2015 at 07:12:49PM +0000, Martyn Welch wrote:
>>>
>>> This patch adds documentation for the chromeos-firmware binding.
>>>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>>> Cc: Kumar Gala <galak@codeaurora.org>
>>> Cc: devicetree@vger.kernel.org
>>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>>> ---
>>>   .../devicetree/bindings/misc/chromeos-firmware.txt | 27
>>> ++++++++++++++++++++++
>>
>>
>> bindings/firmware/ please.
>>
>>>   1 file changed, 27 insertions(+)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>> b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>> new file mode 100644
>>> index 0000000..8240611
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>> @@ -0,0 +1,27 @@
>
>
> <snip>
>
>>> +
>>> +Each signal is represented as a sub-node of "chromeos_firmware":
>>> +Subnode properties:
>>> +
>>> +       - gpios: OF device-tree gpio specification.
>>> +
>>> +Example nodes:
>>> +
>>> +       chromeos_firmware {
>>
>>
>> This should go under /firmware
>>
>
> I've changed this to be:
>
>         firmware {
>                 chromeos {
>                         ...
>                 };
>         ];
>
> Which I generally accept (assuming this is considered a part of the
> firmware) as a better way to represent this in the device tree, however this
> has the nasty side effect of causing the device tree parsing to avoid
> parsing the chromeos child and seeing it's compatible property (as the
> firmware node isn't a bus), resulting in the probe routine not being called.
>
> If I add a 'compatible = "simple-bus"' property to the firmware node it
> works, but this doesn't seem quite right as I believe "simple-bus" is
> defined as a "simple memory mapped bus".
>
> I /could/ rewrite the initialisation to call of_find_compatible_node(), but
> this seems rather hacky and inefficient. I can think of 2 other ways this
> could be resolved:
>
> (1) As this is only tangentially related to firmware, I rename it something
> like "chromeos-signals" and make it it's own node. In essence this driver
> provides a mechanism built on top of specific GPIO (ala gpio-keys seems to
> be, after-all this has a similar use of resources to that).

I'm starting to fail to understand the relationship to firmware here...

gpio-keys are at least a thing (being a key or set of keys). Your
grouping is a rather random collection of GPIOs. Maybe you need
"gpio-switch" binding and then the function would be "label" property.

> (2) Add a compatible string something like 'compatible="logical-group";' to
> the firmware node and add that too the bus matching logic. This would have
> the advantage of solving this in the general case (I guess there are other
> instances where a grouping of things more logically rather than physically
> connected would ideally be grouped together), though I expect there may be
> some strong views regarding this approach.

Why do you need them grouped?

> Would either of those be acceptable or is there a better way of resolving
> this that I've missed?

I don't know as I still don't really understand what the h/w looks like here.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283165

On 03/12/15 15:08, Rob Herring wrote:
> On Thu, Dec 3, 2015 at 4:14 AM, Martyn Welch
> <martyn.welch@collabora.co.uk> wrote:
>>
>> On 02/12/15 15:15, Rob Herring wrote:
>>>
>>> On Tue, Dec 01, 2015 at 07:12:49PM +0000, Martyn Welch wrote:
>>>>
>>>> This patch adds documentation for the chromeos-firmware binding.
>>>>
>>>> Cc: Rob Herring <robh+dt@kernel.org>
>>>> Cc: Pawel Moll <pawel.moll@arm.com>
>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
>>>> Cc: Kumar Gala <galak@codeaurora.org>
>>>> Cc: devicetree@vger.kernel.org
>>>> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
>>>> ---
>>>>    .../devicetree/bindings/misc/chromeos-firmware.txt | 27
>>>> ++++++++++++++++++++++
>>>
>>>
>>> bindings/firmware/ please.
>>>
>>>>    1 file changed, 27 insertions(+)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>> b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>> new file mode 100644
>>>> index 0000000..8240611
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/misc/chromeos-firmware.txt
>>>> @@ -0,0 +1,27 @@
>>
>>
>> <snip>
>>
>>>> +
>>>> +Each signal is represented as a sub-node of "chromeos_firmware":
>>>> +Subnode properties:
>>>> +
>>>> +       - gpios: OF device-tree gpio specification.
>>>> +
>>>> +Example nodes:
>>>> +
>>>> +       chromeos_firmware {
>>>
>>>
>>> This should go under /firmware
>>>
>>
>> I've changed this to be:
>>
>>          firmware {
>>                  chromeos {
>>                          ...
>>                  };
>>          ];
>>
>> Which I generally accept (assuming this is considered a part of the
>> firmware) as a better way to represent this in the device tree, however this
>> has the nasty side effect of causing the device tree parsing to avoid
>> parsing the chromeos child and seeing it's compatible property (as the
>> firmware node isn't a bus), resulting in the probe routine not being called.
>>
>> If I add a 'compatible = "simple-bus"' property to the firmware node it
>> works, but this doesn't seem quite right as I believe "simple-bus" is
>> defined as a "simple memory mapped bus".
>>
>> I /could/ rewrite the initialisation to call of_find_compatible_node(), but
>> this seems rather hacky and inefficient. I can think of 2 other ways this
>> could be resolved:
>>
>> (1) As this is only tangentially related to firmware, I rename it something
>> like "chromeos-signals" and make it it's own node. In essence this driver
>> provides a mechanism built on top of specific GPIO (ala gpio-keys seems to
>> be, after-all this has a similar use of resources to that).
>
> I'm starting to fail to understand the relationship to firmware here...
>
> gpio-keys are at least a thing (being a key or set of keys). Your
> grouping is a rather random collection of GPIOs. Maybe you need
> "gpio-switch" binding and then the function would be "label" property.
>

So, something like this:

gpio-switch {
	compatible = "gpio-switch";

	pinctrl-names = "default";
	pinctrl-0 = <&wp_gpio &dev_mode &rec_mode>;

	write-protect {
		label = "write-protect";
		gpios = <&gpx3 0 GPIO_ACTIVE_LOW>;
		read-only;
	};

	developer-switch {
		label = "developer-switch";
		gpios = <&gpx1 3 GPIO_ACTIVE_HIGH>;
		read-only;
	};

	recovery-switch {
		label = "recovery-switch";
		gpios = <&gpx0 7 GPIO_ACTIVE_LOW>;
		read-only;
	};
};

(Making it much more generic in the process.)


>> (2) Add a compatible string something like 'compatible="logical-group";' to
>> the firmware node and add that too the bus matching logic. This would have
>> the advantage of solving this in the general case (I guess there are other
>> instances where a grouping of things more logically rather than physically
>> connected would ideally be grouped together), though I expect there may be
>> some strong views regarding this approach.
>
> Why do you need them grouped?
>

That's effectively what is achieved by putting this (and I assume 
anything else considered "firmware" under a firmware node isn't it? (or 
and I miss-understanding your request?)

I think it is a moot point, I'll rework as you've suggested.

Martyn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web