[PATCH] firmware: meson-sm: use generic compatible

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

The meson secure monitor seems to be compatible with more SoCs than
initially thought. Let's use the most generic compatible he have in
DT instead of the gxbb specific one

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
 drivers/firmware/meson/meson_sm.c                             | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
index c248cd44f727..13a7815ac2b9 100644
--- a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
+++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
@@ -4,12 +4,12 @@ In the Amlogic SoCs the Secure Monitor code is used to provide access to the
 NVMEM, enable JTAG, set USB boot, etc...
 
 Required properties for the secure monitor node:
-- compatible: Should be "amlogic,meson-gxbb-sm"
+- compatible: Should be "amlogic,meson-gx-sm"
 
 Example:
 
 	firmware {
 		sm: secure-monitor {
-			compatible = "amlogic,meson-gxbb-sm";
+			compatible = "amlogic,meson-gx-sm";
 		};
 	};
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index ff204421117b..c83f7be2b1f4 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -38,7 +38,7 @@ struct meson_sm_chip {
 	struct meson_sm_cmd cmd[];
 };
 
-struct meson_sm_chip gxbb_chip = {
+struct meson_sm_chip gx_chip = {
 	.shmem_size		= SZ_4K,
 	.cmd_shmem_in_base	= 0x82000020,
 	.cmd_shmem_out_base	= 0x82000021,
@@ -213,7 +213,7 @@ int meson_sm_call_write(void *buffer, unsigned int size, unsigned int cmd_index,
 EXPORT_SYMBOL(meson_sm_call_write);
 
 static const struct of_device_id meson_sm_ids[] = {
-	{ .compatible = "amlogic,meson-gxbb-sm", .data = &gxbb_chip },
+	{ .compatible = "amlogic,meson-gx-sm", .data = &gx_chip },
 	{ /* sentinel */ },
 };
 
-- 
2.13.6

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

The meson secure monitor seems to be compatible with more SoCs than
initially thought. Let's use the most generic compatible he have in
DT instead of the gxbb specific one

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
 drivers/firmware/meson/meson_sm.c                             | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
index c248cd44f727..13a7815ac2b9 100644
--- a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
+++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
@@ -4,12 +4,12 @@ In the Amlogic SoCs the Secure Monitor code is used to provide access to the
 NVMEM, enable JTAG, set USB boot, etc...
 
 Required properties for the secure monitor node:
-- compatible: Should be "amlogic,meson-gxbb-sm"
+- compatible: Should be "amlogic,meson-gx-sm"
 
 Example:
 
 	firmware {
 		sm: secure-monitor {
-			compatible = "amlogic,meson-gxbb-sm";
+			compatible = "amlogic,meson-gx-sm";
 		};
 	};
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index ff204421117b..c83f7be2b1f4 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -38,7 +38,7 @@ struct meson_sm_chip {
 	struct meson_sm_cmd cmd[];
 };
 
-struct meson_sm_chip gxbb_chip = {
+struct meson_sm_chip gx_chip = {
 	.shmem_size		= SZ_4K,
 	.cmd_shmem_in_base	= 0x82000020,
 	.cmd_shmem_out_base	= 0x82000021,
@@ -213,7 +213,7 @@ int meson_sm_call_write(void *buffer, unsigned int size, unsigned int cmd_index,
 EXPORT_SYMBOL(meson_sm_call_write);
 
 static const struct of_device_id meson_sm_ids[] = {
-	{ .compatible = "amlogic,meson-gxbb-sm", .data = &gxbb_chip },
+	{ .compatible = "amlogic,meson-gx-sm", .data = &gx_chip },
 	{ /* sentinel */ },
 };
 
-- 
2.13.6

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

The meson secure monitor seems to be compatible with more SoCs than
initially thought. Let's use the most generic compatible he have in
DT instead of the gxbb specific one

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
 drivers/firmware/meson/meson_sm.c                             | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
index c248cd44f727..13a7815ac2b9 100644
--- a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
+++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
@@ -4,12 +4,12 @@ In the Amlogic SoCs the Secure Monitor code is used to provide access to the
 NVMEM, enable JTAG, set USB boot, etc...
 
 Required properties for the secure monitor node:
-- compatible: Should be "amlogic,meson-gxbb-sm"
+- compatible: Should be "amlogic,meson-gx-sm"
 
 Example:
 
 	firmware {
 		sm: secure-monitor {
-			compatible = "amlogic,meson-gxbb-sm";
+			compatible = "amlogic,meson-gx-sm";
 		};
 	};
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index ff204421117b..c83f7be2b1f4 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -38,7 +38,7 @@ struct meson_sm_chip {
 	struct meson_sm_cmd cmd[];
 };
 
-struct meson_sm_chip gxbb_chip = {
+struct meson_sm_chip gx_chip = {
 	.shmem_size		= SZ_4K,
 	.cmd_shmem_in_base	= 0x82000020,
 	.cmd_shmem_out_base	= 0x82000021,
@@ -213,7 +213,7 @@ int meson_sm_call_write(void *buffer, unsigned int size, unsigned int cmd_index,
 EXPORT_SYMBOL(meson_sm_call_write);
 
 static const struct of_device_id meson_sm_ids[] = {
-	{ .compatible = "amlogic,meson-gxbb-sm", .data = &gxbb_chip },
+	{ .compatible = "amlogic,meson-gx-sm", .data = &gx_chip },
 	{ /* sentinel */ },
 };
 
-- 
2.13.6

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

The meson secure monitor seems to be compatible with more SoCs than
initially thought. Let's use the most generic compatible he have in
DT instead of the gxbb specific one

Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
 drivers/firmware/meson/meson_sm.c                             | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
index c248cd44f727..13a7815ac2b9 100644
--- a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
+++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
@@ -4,12 +4,12 @@ In the Amlogic SoCs the Secure Monitor code is used to provide access to the
 NVMEM, enable JTAG, set USB boot, etc...
 
 Required properties for the secure monitor node:
-- compatible: Should be "amlogic,meson-gxbb-sm"
+- compatible: Should be "amlogic,meson-gx-sm"
 
 Example:
 
 	firmware {
 		sm: secure-monitor {
-			compatible = "amlogic,meson-gxbb-sm";
+			compatible = "amlogic,meson-gx-sm";
 		};
 	};
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index ff204421117b..c83f7be2b1f4 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -38,7 +38,7 @@ struct meson_sm_chip {
 	struct meson_sm_cmd cmd[];
 };
 
-struct meson_sm_chip gxbb_chip = {
+struct meson_sm_chip gx_chip = {
 	.shmem_size		= SZ_4K,
 	.cmd_shmem_in_base	= 0x82000020,
 	.cmd_shmem_out_base	= 0x82000021,
@@ -213,7 +213,7 @@ int meson_sm_call_write(void *buffer, unsigned int size, unsigned int cmd_index,
 EXPORT_SYMBOL(meson_sm_call_write);
 
 static const struct of_device_id meson_sm_ids[] = {
-	{ .compatible = "amlogic,meson-gxbb-sm", .data = &gxbb_chip },
+	{ .compatible = "amlogic,meson-gx-sm", .data = &gx_chip },
 	{ /* sentinel */ },
 };
 
-- 
2.13.6

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

Re: [PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> The meson secure monitor seems to be compatible with more SoCs than
> initially thought. Let's use the most generic compatible he have in
> DT instead of the gxbb specific one
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Seems like a pointless, not backwards compatible change to me. In the 
end, it's just a string to match on. Who cares what the string is.

Rob

[PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> The meson secure monitor seems to be compatible with more SoCs than
> initially thought. Let's use the most generic compatible he have in
> DT instead of the gxbb specific one
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Seems like a pointless, not backwards compatible change to me. In the 
end, it's just a string to match on. Who cares what the string is.

Rob

[PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> The meson secure monitor seems to be compatible with more SoCs than
> initially thought. Let's use the most generic compatible he have in
> DT instead of the gxbb specific one
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Seems like a pointless, not backwards compatible change to me. In the 
end, it's just a string to match on. Who cares what the string is.

Rob

Re: [PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> The meson secure monitor seems to be compatible with more SoCs than
> initially thought. Let's use the most generic compatible he have in
> DT instead of the gxbb specific one
> 
> Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Seems like a pointless, not backwards compatible change to me. In the 
end, it's just a string to match on. Who cares what the string is.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Tue, 2017-10-17 at 15:50 -0500, Rob Herring wrote:
> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > The meson secure monitor seems to be compatible with more SoCs than
> > initially thought. Let's use the most generic compatible he have in
> > DT instead of the gxbb specific one
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
> >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> Seems like a pointless, not backwards compatible 

The related DTS have both the SoC family compatible and the SoC specific
compatible. So it is backward compatible (even if this contraints is relaxed on
meson because we are figuring out what is shared between SoCs)

We are asked to put SoC family and SoC specific compatibles in DTS. What's the
point of that if we are going to match on whatever was there first ?

> change to me. In the 
> end, it's just a string to match on. Who cares what the string is.
> 

The matched string has to keep some sort of logic to be maintainable.
It should be clear what the data (or the absence of data) attached to a string
relates to. 

If we really don't care what the string is, we could pick words at random in the
dictionary ... it would be chaos, but at least it would not be confusing 

> Rob

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Tue, 2017-10-17 at 15:50 -0500, Rob Herring wrote:
> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > The meson secure monitor seems to be compatible with more SoCs than
> > initially thought. Let's use the most generic compatible he have in
> > DT instead of the gxbb specific one
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
> >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> Seems like a pointless, not backwards compatible 

The related DTS have both the SoC family compatible and the SoC specific
compatible. So it is backward compatible (even if this contraints is relaxed on
meson because we are figuring out what is shared between SoCs)

We are asked to put SoC family and SoC specific compatibles in DTS. What's the
point of that if we are going to match on whatever was there first ?

> change to me. In the 
> end, it's just a string to match on. Who cares what the string is.
> 

The matched string has to keep some sort of logic to be maintainable.
It should be clear what the data (or the absence of data) attached to a string
relates to. 

If we really don't care what the string is, we could pick words at random in the
dictionary ... it would be chaos, but at least it would not be confusing 

> Rob

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Tue, 2017-10-17 at 15:50 -0500, Rob Herring wrote:
> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > The meson secure monitor seems to be compatible with more SoCs than
> > initially thought. Let's use the most generic compatible he have in
> > DT instead of the gxbb specific one
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
> >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> Seems like a pointless, not backwards compatible 

The related DTS have both the SoC family compatible and the SoC specific
compatible. So it is backward compatible (even if this contraints is relaxed on
meson because we are figuring out what is shared between SoCs)

We are asked to put SoC family and SoC specific compatibles in DTS. What's the
point of that if we are going to match on whatever was there first ?

> change to me. In the 
> end, it's just a string to match on. Who cares what the string is.
> 

The matched string has to keep some sort of logic to be maintainable.
It should be clear what the data (or the absence of data) attached to a string
relates to. 

If we really don't care what the string is, we could pick words at random in the
dictionary ... it would be chaos, but at least it would not be confusing 

> Rob

Re: [PATCH] firmware: meson-sm: use generic compatible

[Kevin Hilman]

Rob Herring <robh@kernel.org> writes:

> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> The meson secure monitor seems to be compatible with more SoCs than
>> initially thought. Let's use the most generic compatible he have in
>> DT instead of the gxbb specific one
>> 
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> Seems like a pointless, not backwards compatible change to me.

I've verified that it's backwards compatible with existing upstream DTs.

> end, it's just a string to match on. Who cares what the string is.

As platform maintiner, I very much care what the strings are and I want
it to be coherent with the platform generic names, and I want the
SoC-specific strings to correspond to the actual SoC names.

Kevin

[PATCH] firmware: meson-sm: use generic compatible

[Kevin Hilman]

Rob Herring <robh@kernel.org> writes:

> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> The meson secure monitor seems to be compatible with more SoCs than
>> initially thought. Let's use the most generic compatible he have in
>> DT instead of the gxbb specific one
>> 
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> Seems like a pointless, not backwards compatible change to me.

I've verified that it's backwards compatible with existing upstream DTs.

> end, it's just a string to match on. Who cares what the string is.

As platform maintiner, I very much care what the strings are and I want
it to be coherent with the platform generic names, and I want the
SoC-specific strings to correspond to the actual SoC names.

Kevin

[PATCH] firmware: meson-sm: use generic compatible

[Kevin Hilman]

Rob Herring <robh@kernel.org> writes:

> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> The meson secure monitor seems to be compatible with more SoCs than
>> initially thought. Let's use the most generic compatible he have in
>> DT instead of the gxbb specific one
>> 
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> Seems like a pointless, not backwards compatible change to me.

I've verified that it's backwards compatible with existing upstream DTs.

> end, it's just a string to match on. Who cares what the string is.

As platform maintiner, I very much care what the strings are and I want
it to be coherent with the platform generic names, and I want the
SoC-specific strings to correspond to the actual SoC names.

Kevin

Re: [PATCH] firmware: meson-sm: use generic compatible

[Kevin Hilman]

Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:

> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> The meson secure monitor seems to be compatible with more SoCs than
>> initially thought. Let's use the most generic compatible he have in
>> DT instead of the gxbb specific one
>> 
>> Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> Seems like a pointless, not backwards compatible change to me.

I've verified that it's backwards compatible with existing upstream DTs.

> end, it's just a string to match on. Who cares what the string is.

As platform maintiner, I very much care what the strings are and I want
it to be coherent with the platform generic names, and I want the
SoC-specific strings to correspond to the actual SoC names.

Kevin



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

Re: [PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>>> The meson secure monitor seems to be compatible with more SoCs than
>>> initially thought. Let's use the most generic compatible he have in
>>> DT instead of the gxbb specific one
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> ---
>>>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>>>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> Seems like a pointless, not backwards compatible change to me.
>
> I've verified that it's backwards compatible with existing upstream DTs.

Perhaps if you all are documenting only what the driver uses, not what
the dts can have as Jerome said.

>> end, it's just a string to match on. Who cares what the string is.
>
> As platform maintiner, I very much care what the strings are and I want
> it to be coherent with the platform generic names, and I want the
> SoC-specific strings to correspond to the actual SoC names.

The most specific compatible should be, absolutely. The fallbacks can
be anything really. Ideally, they are the compatible string for the
1st SoC with "the same" compatible IP. Could be another vendor
entirely even because mergers happen.

Rob

[PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>>> The meson secure monitor seems to be compatible with more SoCs than
>>> initially thought. Let's use the most generic compatible he have in
>>> DT instead of the gxbb specific one
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> ---
>>>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>>>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> Seems like a pointless, not backwards compatible change to me.
>
> I've verified that it's backwards compatible with existing upstream DTs.

Perhaps if you all are documenting only what the driver uses, not what
the dts can have as Jerome said.

>> end, it's just a string to match on. Who cares what the string is.
>
> As platform maintiner, I very much care what the strings are and I want
> it to be coherent with the platform generic names, and I want the
> SoC-specific strings to correspond to the actual SoC names.

The most specific compatible should be, absolutely. The fallbacks can
be anything really. Ideally, they are the compatible string for the
1st SoC with "the same" compatible IP. Could be another vendor
entirely even because mergers happen.

Rob

[PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>>> The meson secure monitor seems to be compatible with more SoCs than
>>> initially thought. Let's use the most generic compatible he have in
>>> DT instead of the gxbb specific one
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> ---
>>>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>>>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> Seems like a pointless, not backwards compatible change to me.
>
> I've verified that it's backwards compatible with existing upstream DTs.

Perhaps if you all are documenting only what the driver uses, not what
the dts can have as Jerome said.

>> end, it's just a string to match on. Who cares what the string is.
>
> As platform maintiner, I very much care what the strings are and I want
> it to be coherent with the platform generic names, and I want the
> SoC-specific strings to correspond to the actual SoC names.

The most specific compatible should be, absolutely. The fallbacks can
be anything really. Ideally, they are the compatible string for the
1st SoC with "the same" compatible IP. Could be another vendor
entirely even because mergers happen.

Rob

Re: [PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
> Rob Herring <robh@kernel.org> writes:
>
>> On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>>> The meson secure monitor seems to be compatible with more SoCs than
>>> initially thought. Let's use the most generic compatible he have in
>>> DT instead of the gxbb specific one
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> ---
>>>  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>>>  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> Seems like a pointless, not backwards compatible change to me.
>
> I've verified that it's backwards compatible with existing upstream DTs.

Perhaps if you all are documenting only what the driver uses, not what
the dts can have as Jerome said.

>> end, it's just a string to match on. Who cares what the string is.
>
> As platform maintiner, I very much care what the strings are and I want
> it to be coherent with the platform generic names, and I want the
> SoC-specific strings to correspond to the actual SoC names.

The most specific compatible should be, absolutely. The fallbacks can
be anything really. Ideally, they are the compatible string for the
1st SoC with "the same" compatible IP. Could be another vendor
entirely even because mergers happen.

Rob

Re: [PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
> On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
> > Rob Herring <robh@kernel.org> writes:
> > 
> > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > > > The meson secure monitor seems to be compatible with more SoCs than
> > > > initially thought. Let's use the most generic compatible he have in
> > > > DT instead of the gxbb specific one
> > > > 
> > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
> > > >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > Seems like a pointless, not backwards compatible change to me.
> > 
> > I've verified that it's backwards compatible with existing upstream DTs.
> 
> Perhaps if you all are documenting only what the driver uses, not what
> the dts can have as Jerome said.
> 
> > > end, it's just a string to match on. Who cares what the string is.
> > 
> > As platform maintiner, I very much care what the strings are and I want
> > it to be coherent with the platform generic names, and I want the
> > SoC-specific strings to correspond to the actual SoC names.
> 
> The most specific compatible should be, absolutely. The fallbacks can
> be anything really. Ideally, they are the compatible string for the
> 1st SoC with "the same" compatible IP. Could be another vendor
> entirely even because mergers happen.

Then what's your problem with these patches again ?

I am just asking the driver to match the generic binding instead of the SoC
specific, because we are also using it on other SoC, as explain in the patch
comment. Does not seems that "pointless" to me.

Right now the driver match only on: vendor,soc-one
in dts, we have compatible = "vendor,family", "vendor,soc-one" 

but it is compatible with soc-two as well.
to match we would have to put "vendor,soc-one" as well which is a mess

By expressing correctly what the driver is compatible with, "vendor,family"
we can dts that makes sense for soc-two as well
compatible = "vendor,family", "vendor,soc-two"

Same goes for the other patches

> 
> Rob

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
> On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
> > Rob Herring <robh@kernel.org> writes:
> > 
> > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > > > The meson secure monitor seems to be compatible with more SoCs than
> > > > initially thought. Let's use the most generic compatible he have in
> > > > DT instead of the gxbb specific one
> > > > 
> > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
> > > >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > Seems like a pointless, not backwards compatible change to me.
> > 
> > I've verified that it's backwards compatible with existing upstream DTs.
> 
> Perhaps if you all are documenting only what the driver uses, not what
> the dts can have as Jerome said.
> 
> > > end, it's just a string to match on. Who cares what the string is.
> > 
> > As platform maintiner, I very much care what the strings are and I want
> > it to be coherent with the platform generic names, and I want the
> > SoC-specific strings to correspond to the actual SoC names.
> 
> The most specific compatible should be, absolutely. The fallbacks can
> be anything really. Ideally, they are the compatible string for the
> 1st SoC with "the same" compatible IP. Could be another vendor
> entirely even because mergers happen.

Then what's your problem with these patches again ?

I am just asking the driver to match the generic binding instead of the SoC
specific, because we are also using it on other SoC, as explain in the patch
comment. Does not seems that "pointless" to me.

Right now the driver match only on: vendor,soc-one
in dts, we have compatible = "vendor,family", "vendor,soc-one" 

but it is compatible with soc-two as well.
to match we would have to put "vendor,soc-one" as well which is a mess

By expressing correctly what the driver is compatible with, "vendor,family"
we can dts that makes sense for soc-two as well
compatible = "vendor,family", "vendor,soc-two"

Same goes for the other patches

> 
> Rob

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
> On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
> > Rob Herring <robh@kernel.org> writes:
> > 
> > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > > > The meson secure monitor seems to be compatible with more SoCs than
> > > > initially thought. Let's use the most generic compatible he have in
> > > > DT instead of the gxbb specific one
> > > > 
> > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
> > > >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > Seems like a pointless, not backwards compatible change to me.
> > 
> > I've verified that it's backwards compatible with existing upstream DTs.
> 
> Perhaps if you all are documenting only what the driver uses, not what
> the dts can have as Jerome said.
> 
> > > end, it's just a string to match on. Who cares what the string is.
> > 
> > As platform maintiner, I very much care what the strings are and I want
> > it to be coherent with the platform generic names, and I want the
> > SoC-specific strings to correspond to the actual SoC names.
> 
> The most specific compatible should be, absolutely. The fallbacks can
> be anything really. Ideally, they are the compatible string for the
> 1st SoC with "the same" compatible IP. Could be another vendor
> entirely even because mergers happen.

Then what's your problem with these patches again ?

I am just asking the driver to match the generic binding instead of the SoC
specific, because we are also using it on other SoC, as explain in the patch
comment. Does not seems that "pointless" to me.

Right now the driver match only on: vendor,soc-one
in dts, we have compatible = "vendor,family", "vendor,soc-one" 

but it is compatible with soc-two as well.
to match we would have to put "vendor,soc-one" as well which is a mess

By expressing correctly what the driver is compatible with, "vendor,family"
we can dts that makes sense for soc-two as well
compatible = "vendor,family", "vendor,soc-two"

Same goes for the other patches

> 
> Rob

Re: [PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
> On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> > Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
> > 
> > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > > > The meson secure monitor seems to be compatible with more SoCs than
> > > > initially thought. Let's use the most generic compatible he have in
> > > > DT instead of the gxbb specific one
> > > > 
> > > > Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
> > > >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > Seems like a pointless, not backwards compatible change to me.
> > 
> > I've verified that it's backwards compatible with existing upstream DTs.
> 
> Perhaps if you all are documenting only what the driver uses, not what
> the dts can have as Jerome said.
> 
> > > end, it's just a string to match on. Who cares what the string is.
> > 
> > As platform maintiner, I very much care what the strings are and I want
> > it to be coherent with the platform generic names, and I want the
> > SoC-specific strings to correspond to the actual SoC names.
> 
> The most specific compatible should be, absolutely. The fallbacks can
> be anything really. Ideally, they are the compatible string for the
> 1st SoC with "the same" compatible IP. Could be another vendor
> entirely even because mergers happen.

Then what's your problem with these patches again ?

I am just asking the driver to match the generic binding instead of the SoC
specific, because we are also using it on other SoC, as explain in the patch
comment. Does not seems that "pointless" to me.

Right now the driver match only on: vendor,soc-one
in dts, we have compatible = "vendor,family", "vendor,soc-one" 

but it is compatible with soc-two as well.
to match we would have to put "vendor,soc-one" as well which is a mess

By expressing correctly what the driver is compatible with, "vendor,family"
we can dts that makes sense for soc-two as well
compatible = "vendor,family", "vendor,soc-two"

Same goes for the other patches

> 
> Rob

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

Re: [PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
>> On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
>> > Rob Herring <robh@kernel.org> writes:
>> >
>> > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> > > > The meson secure monitor seems to be compatible with more SoCs than
>> > > > initially thought. Let's use the most generic compatible he have in
>> > > > DT instead of the gxbb specific one
>> > > >
>> > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> > > > ---
>> > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>> > > >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
>> > >
>> > > Seems like a pointless, not backwards compatible change to me.
>> >
>> > I've verified that it's backwards compatible with existing upstream DTs.
>>
>> Perhaps if you all are documenting only what the driver uses, not what
>> the dts can have as Jerome said.
>>
>> > > end, it's just a string to match on. Who cares what the string is.
>> >
>> > As platform maintiner, I very much care what the strings are and I want
>> > it to be coherent with the platform generic names, and I want the
>> > SoC-specific strings to correspond to the actual SoC names.
>>
>> The most specific compatible should be, absolutely. The fallbacks can
>> be anything really. Ideally, they are the compatible string for the
>> 1st SoC with "the same" compatible IP. Could be another vendor
>> entirely even because mergers happen.
>
> Then what's your problem with these patches again ?

Removal of the SoC specific compatible and breaking compatibility.
Kevin says compatibility is not broken, but it obviously it based on
the example and the driver change. So that can only mean your dts file
doesn't match the example.

> I am just asking the driver to match the generic binding instead of the SoC
> specific, because we are also using it on other SoC, as explain in the patch
> comment. Does not seems that "pointless" to me.
>
> Right now the driver match only on: vendor,soc-one
> in dts, we have compatible = "vendor,family", "vendor,soc-one"

That's backwards if I understand this right. It should be most
specific first, but that's a separate issue.

> but it is compatible with soc-two as well.
> to match we would have to put "vendor,soc-one" as well which is a mess

Why? That's how DT works and every other platform follows. Either you have:

"vendor,soc-one", "vendor,family"
"vendor,soc-two", "vendor,family"

or

"vendor,soc-one"
"vendor,soc-two", "vendor,soc-one"

The latter is how DT has existed and worked for 20+ years. The former
is what we allow because for some reason people have such an aversion
to saying soc2 is compatible with soc1.

Either one is fine, but the documentation must be clear what the
constraints are for the dts file. For example, "vendor,soc-two" or
"vendor,family" alone are not valid. There's plenty of examples to
follow. Renesas is one using "vendor,family" extensively.

> By expressing correctly what the driver is compatible with, "vendor,family"
> we can dts that makes sense for soc-two as well
> compatible = "vendor,family", "vendor,soc-two"

Binding docs may live in the kernel, but they are separate from
drivers. They describe the constraints of the dts files. There's no
reason to document what the driver wants because I can read the driver
source. I can't say the same thing about the dts file. The dts may not
come from the kernel and one dts file is not all possible options for
a given binding.

Rob

[PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
>> On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
>> > Rob Herring <robh@kernel.org> writes:
>> >
>> > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> > > > The meson secure monitor seems to be compatible with more SoCs than
>> > > > initially thought. Let's use the most generic compatible he have in
>> > > > DT instead of the gxbb specific one
>> > > >
>> > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> > > > ---
>> > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>> > > >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
>> > >
>> > > Seems like a pointless, not backwards compatible change to me.
>> >
>> > I've verified that it's backwards compatible with existing upstream DTs.
>>
>> Perhaps if you all are documenting only what the driver uses, not what
>> the dts can have as Jerome said.
>>
>> > > end, it's just a string to match on. Who cares what the string is.
>> >
>> > As platform maintiner, I very much care what the strings are and I want
>> > it to be coherent with the platform generic names, and I want the
>> > SoC-specific strings to correspond to the actual SoC names.
>>
>> The most specific compatible should be, absolutely. The fallbacks can
>> be anything really. Ideally, they are the compatible string for the
>> 1st SoC with "the same" compatible IP. Could be another vendor
>> entirely even because mergers happen.
>
> Then what's your problem with these patches again ?

Removal of the SoC specific compatible and breaking compatibility.
Kevin says compatibility is not broken, but it obviously it based on
the example and the driver change. So that can only mean your dts file
doesn't match the example.

> I am just asking the driver to match the generic binding instead of the SoC
> specific, because we are also using it on other SoC, as explain in the patch
> comment. Does not seems that "pointless" to me.
>
> Right now the driver match only on: vendor,soc-one
> in dts, we have compatible = "vendor,family", "vendor,soc-one"

That's backwards if I understand this right. It should be most
specific first, but that's a separate issue.

> but it is compatible with soc-two as well.
> to match we would have to put "vendor,soc-one" as well which is a mess

Why? That's how DT works and every other platform follows. Either you have:

"vendor,soc-one", "vendor,family"
"vendor,soc-two", "vendor,family"

or

"vendor,soc-one"
"vendor,soc-two", "vendor,soc-one"

The latter is how DT has existed and worked for 20+ years. The former
is what we allow because for some reason people have such an aversion
to saying soc2 is compatible with soc1.

Either one is fine, but the documentation must be clear what the
constraints are for the dts file. For example, "vendor,soc-two" or
"vendor,family" alone are not valid. There's plenty of examples to
follow. Renesas is one using "vendor,family" extensively.

> By expressing correctly what the driver is compatible with, "vendor,family"
> we can dts that makes sense for soc-two as well
> compatible = "vendor,family", "vendor,soc-two"

Binding docs may live in the kernel, but they are separate from
drivers. They describe the constraints of the dts files. There's no
reason to document what the driver wants because I can read the driver
source. I can't say the same thing about the dts file. The dts may not
come from the kernel and one dts file is not all possible options for
a given binding.

Rob

[PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
>> On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
>> > Rob Herring <robh@kernel.org> writes:
>> >
>> > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> > > > The meson secure monitor seems to be compatible with more SoCs than
>> > > > initially thought. Let's use the most generic compatible he have in
>> > > > DT instead of the gxbb specific one
>> > > >
>> > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> > > > ---
>> > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>> > > >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
>> > >
>> > > Seems like a pointless, not backwards compatible change to me.
>> >
>> > I've verified that it's backwards compatible with existing upstream DTs.
>>
>> Perhaps if you all are documenting only what the driver uses, not what
>> the dts can have as Jerome said.
>>
>> > > end, it's just a string to match on. Who cares what the string is.
>> >
>> > As platform maintiner, I very much care what the strings are and I want
>> > it to be coherent with the platform generic names, and I want the
>> > SoC-specific strings to correspond to the actual SoC names.
>>
>> The most specific compatible should be, absolutely. The fallbacks can
>> be anything really. Ideally, they are the compatible string for the
>> 1st SoC with "the same" compatible IP. Could be another vendor
>> entirely even because mergers happen.
>
> Then what's your problem with these patches again ?

Removal of the SoC specific compatible and breaking compatibility.
Kevin says compatibility is not broken, but it obviously it based on
the example and the driver change. So that can only mean your dts file
doesn't match the example.

> I am just asking the driver to match the generic binding instead of the SoC
> specific, because we are also using it on other SoC, as explain in the patch
> comment. Does not seems that "pointless" to me.
>
> Right now the driver match only on: vendor,soc-one
> in dts, we have compatible = "vendor,family", "vendor,soc-one"

That's backwards if I understand this right. It should be most
specific first, but that's a separate issue.

> but it is compatible with soc-two as well.
> to match we would have to put "vendor,soc-one" as well which is a mess

Why? That's how DT works and every other platform follows. Either you have:

"vendor,soc-one", "vendor,family"
"vendor,soc-two", "vendor,family"

or

"vendor,soc-one"
"vendor,soc-two", "vendor,soc-one"

The latter is how DT has existed and worked for 20+ years. The former
is what we allow because for some reason people have such an aversion
to saying soc2 is compatible with soc1.

Either one is fine, but the documentation must be clear what the
constraints are for the dts file. For example, "vendor,soc-two" or
"vendor,family" alone are not valid. There's plenty of examples to
follow. Renesas is one using "vendor,family" extensively.

> By expressing correctly what the driver is compatible with, "vendor,family"
> we can dts that makes sense for soc-two as well
> compatible = "vendor,family", "vendor,soc-two"

Binding docs may live in the kernel, but they are separate from
drivers. They describe the constraints of the dts files. There's no
reason to document what the driver wants because I can read the driver
source. I can't say the same thing about the dts file. The dts may not
come from the kernel and one dts file is not all possible options for
a given binding.

Rob

Re: [PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
>> On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com> wrote:
>> > Rob Herring <robh@kernel.org> writes:
>> >
>> > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> > > > The meson secure monitor seems to be compatible with more SoCs than
>> > > > initially thought. Let's use the most generic compatible he have in
>> > > > DT instead of the gxbb specific one
>> > > >
>> > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> > > > ---
>> > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4 ++--
>> > > >  drivers/firmware/meson/meson_sm.c                             | 4 ++--
>> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
>> > >
>> > > Seems like a pointless, not backwards compatible change to me.
>> >
>> > I've verified that it's backwards compatible with existing upstream DTs.
>>
>> Perhaps if you all are documenting only what the driver uses, not what
>> the dts can have as Jerome said.
>>
>> > > end, it's just a string to match on. Who cares what the string is.
>> >
>> > As platform maintiner, I very much care what the strings are and I want
>> > it to be coherent with the platform generic names, and I want the
>> > SoC-specific strings to correspond to the actual SoC names.
>>
>> The most specific compatible should be, absolutely. The fallbacks can
>> be anything really. Ideally, they are the compatible string for the
>> 1st SoC with "the same" compatible IP. Could be another vendor
>> entirely even because mergers happen.
>
> Then what's your problem with these patches again ?

Removal of the SoC specific compatible and breaking compatibility.
Kevin says compatibility is not broken, but it obviously it based on
the example and the driver change. So that can only mean your dts file
doesn't match the example.

> I am just asking the driver to match the generic binding instead of the SoC
> specific, because we are also using it on other SoC, as explain in the patch
> comment. Does not seems that "pointless" to me.
>
> Right now the driver match only on: vendor,soc-one
> in dts, we have compatible = "vendor,family", "vendor,soc-one"

That's backwards if I understand this right. It should be most
specific first, but that's a separate issue.

> but it is compatible with soc-two as well.
> to match we would have to put "vendor,soc-one" as well which is a mess

Why? That's how DT works and every other platform follows. Either you have:

"vendor,soc-one", "vendor,family"
"vendor,soc-two", "vendor,family"

or

"vendor,soc-one"
"vendor,soc-two", "vendor,soc-one"

The latter is how DT has existed and worked for 20+ years. The former
is what we allow because for some reason people have such an aversion
to saying soc2 is compatible with soc1.

Either one is fine, but the documentation must be clear what the
constraints are for the dts file. For example, "vendor,soc-two" or
"vendor,family" alone are not valid. There's plenty of examples to
follow. Renesas is one using "vendor,family" extensively.

> By expressing correctly what the driver is compatible with, "vendor,family"
> we can dts that makes sense for soc-two as well
> compatible = "vendor,family", "vendor,soc-two"

Binding docs may live in the kernel, but they are separate from
drivers. They describe the constraints of the dts files. There's no
reason to document what the driver wants because I can read the driver
source. I can't say the same thing about the dts file. The dts may not
come from the kernel and one dts file is not all possible options for
a given binding.

Rob

Re: [PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Fri, 2017-10-20 at 14:34 -0500, Rob Herring wrote:
> On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> > On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
> > > On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com>
> > > wrote:
> > > > Rob Herring <robh@kernel.org> writes:
> > > > 
> > > > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > > > > > The meson secure monitor seems to be compatible with more SoCs than
> > > > > > initially thought. Let's use the most generic compatible he have in
> > > > > > DT instead of the gxbb specific one
> > > > > > 
> > > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4
> > > > > > ++--
> > > > > >  drivers/firmware/meson/meson_sm.c                             | 4
> > > > > > ++--
> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > > > 
> > > > > Seems like a pointless, not backwards compatible change to me.
> > > > 
> > > > I've verified that it's backwards compatible with existing upstream DTs.
> > > 
> > > Perhaps if you all are documenting only what the driver uses, not what
> > > the dts can have as Jerome said.
> > > 
> > > > > end, it's just a string to match on. Who cares what the string is.
> > > > 
> > > > As platform maintiner, I very much care what the strings are and I want
> > > > it to be coherent with the platform generic names, and I want the
> > > > SoC-specific strings to correspond to the actual SoC names.
> > > 
> > > The most specific compatible should be, absolutely. The fallbacks can
> > > be anything really. Ideally, they are the compatible string for the
> > > 1st SoC with "the same" compatible IP. Could be another vendor
> > > entirely even because mergers happen.
> > 
> > Then what's your problem with these patches again ?
> 
> Removal of the SoC specific compatible and breaking compatibility.
> Kevin says compatibility is not broken, but it obviously it based on
> the example and the driver change. So that can only mean your dts file
> doesn't match the example.

I believe it does, but I suppose I have missed something.

Currently the driver only matches on:
.compatible = "amlogic,meson-gxbb-sm" (drivers/firmware/meson_sm.c)

The only device-tree file using it is 
meson-gx.dtsi: compatible = "amlogic,meson-gx-sm", "amlogic,meson-gxbb-sm";

So:
1) yes the order is backward (thx for pointing this out)
2) by changing the compatible matched by the driver from "amlogic,meson-gxbb-sm" 
to "amlogic,meson-gx-sm", I don't think I am breaking anything. Feel free to
point out why this is wrong because I don't get it. 

> 
> > I am just asking the driver to match the generic binding instead of the SoC
> > specific, because we are also using it on other SoC, as explain in the patch
> > comment. Does not seems that "pointless" to me.
> > 
> > Right now the driver match only on: vendor,soc-one
> > in dts, we have compatible = "vendor,family", "vendor,soc-one"
> 
> That's backwards if I understand this right. It should be most
> specific first, but that's a separate issue.
> 
> > but it is compatible with soc-two as well.
> > to match we would have to put "vendor,soc-one" as well which is a mess
> 
> Why? That's how DT works and every other platform follows. Either you have:
> 
> "vendor,soc-one", "vendor,family"
> "vendor,soc-two", "vendor,family"
> 
> or
> 
> "vendor,soc-one"
> "vendor,soc-two", "vendor,soc-one"
> 
> The latter is how DT has existed and worked for 20+ years. The former
> is what we allow because for some reason people have such an aversion
> to saying soc2 is compatible with soc1.
> 
> Either one is fine, but the documentation must be clear what the
> constraints are for the dts file. For example, "vendor,soc-two" or
> "vendor,family" alone are not valid. There's plenty of examples to

except that the DT file where it is used is a soc family dtsi.
meson-gx.dtsi is the common DT for gxbb and gxl family. Wouldn't it make sense 
to have the SoC generic compatible alone here ? and override it in the SoC DT if
necessary ? 

> follow. Renesas is one using "vendor,family" extensively.
> 
> > By expressing correctly what the driver is compatible with, "vendor,family"
> > we can dts that makes sense for soc-two as well
> > compatible = "vendor,family", "vendor,soc-two"

Honestly, I don't care which of the 2 solutions we take. The important thing for
me is to eliminate the confusion introduced by the generic-compatible being here
and useless. The driver is indeed generic, but the related compatible is not
matched. This makes no sense.

So either:
* we use "vendor,soc-one", "vendor,family" model: I believe this what I'm trying
 to do here. For this, we need to match the soc generic compatible in the
driver.

* we use the "vendor,soc-two", "vendor,soc-one" model : The driver keeps on
matching the SoC specific compatible. We won't ever match the generic one with
this model, what is the point of keeping it around in DT ? Should we remove it ?

Maybe the later is better, it would end up just being a removal of an
undocumented property from DT.

> 
> Binding docs may live in the kernel, but they are separate from
> drivers. They describe the constraints of the dts files. There's no
> reason to document what the driver wants because I can read the driver
> source. I can't say the same thing about the dts file. The dts may not
> come from the kernel and one dts file is not all possible options for
> a given binding.
> 
> Rob

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Fri, 2017-10-20 at 14:34 -0500, Rob Herring wrote:
> On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> > On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
> > > On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com>
> > > wrote:
> > > > Rob Herring <robh@kernel.org> writes:
> > > > 
> > > > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > > > > > The meson secure monitor seems to be compatible with more SoCs than
> > > > > > initially thought. Let's use the most generic compatible he have in
> > > > > > DT instead of the gxbb specific one
> > > > > > 
> > > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4
> > > > > > ++--
> > > > > >  drivers/firmware/meson/meson_sm.c                             | 4
> > > > > > ++--
> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > > > 
> > > > > Seems like a pointless, not backwards compatible change to me.
> > > > 
> > > > I've verified that it's backwards compatible with existing upstream DTs.
> > > 
> > > Perhaps if you all are documenting only what the driver uses, not what
> > > the dts can have as Jerome said.
> > > 
> > > > > end, it's just a string to match on. Who cares what the string is.
> > > > 
> > > > As platform maintiner, I very much care what the strings are and I want
> > > > it to be coherent with the platform generic names, and I want the
> > > > SoC-specific strings to correspond to the actual SoC names.
> > > 
> > > The most specific compatible should be, absolutely. The fallbacks can
> > > be anything really. Ideally, they are the compatible string for the
> > > 1st SoC with "the same" compatible IP. Could be another vendor
> > > entirely even because mergers happen.
> > 
> > Then what's your problem with these patches again ?
> 
> Removal of the SoC specific compatible and breaking compatibility.
> Kevin says compatibility is not broken, but it obviously it based on
> the example and the driver change. So that can only mean your dts file
> doesn't match the example.

I believe it does, but I suppose I have missed something.

Currently the driver only matches on:
.compatible = "amlogic,meson-gxbb-sm" (drivers/firmware/meson_sm.c)

The only device-tree file using it is 
meson-gx.dtsi: compatible = "amlogic,meson-gx-sm", "amlogic,meson-gxbb-sm";

So:
1) yes the order is backward (thx for pointing this out)
2) by changing the compatible matched by the driver from "amlogic,meson-gxbb-sm" 
to "amlogic,meson-gx-sm", I don't think I am breaking anything. Feel free to
point out why this is wrong because I don't get it. 

> 
> > I am just asking the driver to match the generic binding instead of the SoC
> > specific, because we are also using it on other SoC, as explain in the patch
> > comment. Does not seems that "pointless" to me.
> > 
> > Right now the driver match only on: vendor,soc-one
> > in dts, we have compatible = "vendor,family", "vendor,soc-one"
> 
> That's backwards if I understand this right. It should be most
> specific first, but that's a separate issue.
> 
> > but it is compatible with soc-two as well.
> > to match we would have to put "vendor,soc-one" as well which is a mess
> 
> Why? That's how DT works and every other platform follows. Either you have:
> 
> "vendor,soc-one", "vendor,family"
> "vendor,soc-two", "vendor,family"
> 
> or
> 
> "vendor,soc-one"
> "vendor,soc-two", "vendor,soc-one"
> 
> The latter is how DT has existed and worked for 20+ years. The former
> is what we allow because for some reason people have such an aversion
> to saying soc2 is compatible with soc1.
> 
> Either one is fine, but the documentation must be clear what the
> constraints are for the dts file. For example, "vendor,soc-two" or
> "vendor,family" alone are not valid. There's plenty of examples to

except that the DT file where it is used is a soc family dtsi.
meson-gx.dtsi is the common DT for gxbb and gxl family. Wouldn't it make sense 
to have the SoC generic compatible alone here ? and override it in the SoC DT if
necessary ? 

> follow. Renesas is one using "vendor,family" extensively.
> 
> > By expressing correctly what the driver is compatible with, "vendor,family"
> > we can dts that makes sense for soc-two as well
> > compatible = "vendor,family", "vendor,soc-two"

Honestly, I don't care which of the 2 solutions we take. The important thing for
me is to eliminate the confusion introduced by the generic-compatible being here
and useless. The driver is indeed generic, but the related compatible is not
matched. This makes no sense.

So either:
* we use "vendor,soc-one", "vendor,family" model: I believe this what I'm trying
 to do here. For this, we need to match the soc generic compatible in the
driver.

* we use the "vendor,soc-two", "vendor,soc-one" model : The driver keeps on
matching the SoC specific compatible. We won't ever match the generic one with
this model, what is the point of keeping it around in DT ? Should we remove it ?

Maybe the later is better, it would end up just being a removal of an
undocumented property from DT.

> 
> Binding docs may live in the kernel, but they are separate from
> drivers. They describe the constraints of the dts files. There's no
> reason to document what the driver wants because I can read the driver
> source. I can't say the same thing about the dts file. The dts may not
> come from the kernel and one dts file is not all possible options for
> a given binding.
> 
> Rob

[PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Fri, 2017-10-20 at 14:34 -0500, Rob Herring wrote:
> On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> > On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
> > > On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com>
> > > wrote:
> > > > Rob Herring <robh@kernel.org> writes:
> > > > 
> > > > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > > > > > The meson secure monitor seems to be compatible with more SoCs than
> > > > > > initially thought. Let's use the most generic compatible he have in
> > > > > > DT instead of the gxbb specific one
> > > > > > 
> > > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4
> > > > > > ++--
> > > > > >  drivers/firmware/meson/meson_sm.c                             | 4
> > > > > > ++--
> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > > > 
> > > > > Seems like a pointless, not backwards compatible change to me.
> > > > 
> > > > I've verified that it's backwards compatible with existing upstream DTs.
> > > 
> > > Perhaps if you all are documenting only what the driver uses, not what
> > > the dts can have as Jerome said.
> > > 
> > > > > end, it's just a string to match on. Who cares what the string is.
> > > > 
> > > > As platform maintiner, I very much care what the strings are and I want
> > > > it to be coherent with the platform generic names, and I want the
> > > > SoC-specific strings to correspond to the actual SoC names.
> > > 
> > > The most specific compatible should be, absolutely. The fallbacks can
> > > be anything really. Ideally, they are the compatible string for the
> > > 1st SoC with "the same" compatible IP. Could be another vendor
> > > entirely even because mergers happen.
> > 
> > Then what's your problem with these patches again ?
> 
> Removal of the SoC specific compatible and breaking compatibility.
> Kevin says compatibility is not broken, but it obviously it based on
> the example and the driver change. So that can only mean your dts file
> doesn't match the example.

I believe it does, but I suppose I have missed something.

Currently the driver only matches on:
.compatible = "amlogic,meson-gxbb-sm" (drivers/firmware/meson_sm.c)

The only device-tree file using it is 
meson-gx.dtsi: compatible = "amlogic,meson-gx-sm", "amlogic,meson-gxbb-sm";

So:
1) yes the order is backward (thx for pointing this out)
2) by changing the compatible matched by the driver from "amlogic,meson-gxbb-sm" 
to "amlogic,meson-gx-sm", I don't think I am breaking anything. Feel free to
point out why this is wrong because I don't get it. 

> 
> > I am just asking the driver to match the generic binding instead of the SoC
> > specific, because we are also using it on other SoC, as explain in the patch
> > comment. Does not seems that "pointless" to me.
> > 
> > Right now the driver match only on: vendor,soc-one
> > in dts, we have compatible = "vendor,family", "vendor,soc-one"
> 
> That's backwards if I understand this right. It should be most
> specific first, but that's a separate issue.
> 
> > but it is compatible with soc-two as well.
> > to match we would have to put "vendor,soc-one" as well which is a mess
> 
> Why? That's how DT works and every other platform follows. Either you have:
> 
> "vendor,soc-one", "vendor,family"
> "vendor,soc-two", "vendor,family"
> 
> or
> 
> "vendor,soc-one"
> "vendor,soc-two", "vendor,soc-one"
> 
> The latter is how DT has existed and worked for 20+ years. The former
> is what we allow because for some reason people have such an aversion
> to saying soc2 is compatible with soc1.
> 
> Either one is fine, but the documentation must be clear what the
> constraints are for the dts file. For example, "vendor,soc-two" or
> "vendor,family" alone are not valid. There's plenty of examples to

except that the DT file where it is used is a soc family dtsi.
meson-gx.dtsi is the common DT for gxbb and gxl family. Wouldn't it make sense 
to have the SoC generic compatible alone here ? and override it in the SoC DT if
necessary ? 

> follow. Renesas is one using "vendor,family" extensively.
> 
> > By expressing correctly what the driver is compatible with, "vendor,family"
> > we can dts that makes sense for soc-two as well
> > compatible = "vendor,family", "vendor,soc-two"

Honestly, I don't care which of the 2 solutions we take. The important thing for
me is to eliminate the confusion introduced by the generic-compatible being here
and useless. The driver is indeed generic, but the related compatible is not
matched. This makes no sense.

So either:
* we use "vendor,soc-one", "vendor,family" model: I believe this what I'm trying
 to do here. For this, we need to match the soc generic compatible in the
driver.

* we use the "vendor,soc-two", "vendor,soc-one" model : The driver keeps on
matching the SoC specific compatible. We won't ever match the generic one with
this model, what is the point of keeping it around in DT ? Should we remove it ?

Maybe the later is better, it would end up just being a removal of an
undocumented property from DT.

> 
> Binding docs may live in the kernel, but they are separate from
> drivers. They describe the constraints of the dts files. There's no
> reason to document what the driver wants because I can read the driver
> source. I can't say the same thing about the dts file. The dts may not
> come from the kernel and one dts file is not all possible options for
> a given binding.
> 
> Rob

Re: [PATCH] firmware: meson-sm: use generic compatible

[Jerome Brunet]

On Fri, 2017-10-20 at 14:34 -0500, Rob Herring wrote:
> On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> > On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
> > > On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > > wrote:
> > > > Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
> > > > 
> > > > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
> > > > > > The meson secure monitor seems to be compatible with more SoCs than
> > > > > > initially thought. Let's use the most generic compatible he have in
> > > > > > DT instead of the gxbb specific one
> > > > > > 
> > > > > > Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4
> > > > > > ++--
> > > > > >  drivers/firmware/meson/meson_sm.c                             | 4
> > > > > > ++--
> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > > > 
> > > > > Seems like a pointless, not backwards compatible change to me.
> > > > 
> > > > I've verified that it's backwards compatible with existing upstream DTs.
> > > 
> > > Perhaps if you all are documenting only what the driver uses, not what
> > > the dts can have as Jerome said.
> > > 
> > > > > end, it's just a string to match on. Who cares what the string is.
> > > > 
> > > > As platform maintiner, I very much care what the strings are and I want
> > > > it to be coherent with the platform generic names, and I want the
> > > > SoC-specific strings to correspond to the actual SoC names.
> > > 
> > > The most specific compatible should be, absolutely. The fallbacks can
> > > be anything really. Ideally, they are the compatible string for the
> > > 1st SoC with "the same" compatible IP. Could be another vendor
> > > entirely even because mergers happen.
> > 
> > Then what's your problem with these patches again ?
> 
> Removal of the SoC specific compatible and breaking compatibility.
> Kevin says compatibility is not broken, but it obviously it based on
> the example and the driver change. So that can only mean your dts file
> doesn't match the example.

I believe it does, but I suppose I have missed something.

Currently the driver only matches on:
.compatible = "amlogic,meson-gxbb-sm" (drivers/firmware/meson_sm.c)

The only device-tree file using it is 
meson-gx.dtsi: compatible = "amlogic,meson-gx-sm", "amlogic,meson-gxbb-sm";

So:
1) yes the order is backward (thx for pointing this out)
2) by changing the compatible matched by the driver from "amlogic,meson-gxbb-sm" 
to "amlogic,meson-gx-sm", I don't think I am breaking anything. Feel free to
point out why this is wrong because I don't get it. 

> 
> > I am just asking the driver to match the generic binding instead of the SoC
> > specific, because we are also using it on other SoC, as explain in the patch
> > comment. Does not seems that "pointless" to me.
> > 
> > Right now the driver match only on: vendor,soc-one
> > in dts, we have compatible = "vendor,family", "vendor,soc-one"
> 
> That's backwards if I understand this right. It should be most
> specific first, but that's a separate issue.
> 
> > but it is compatible with soc-two as well.
> > to match we would have to put "vendor,soc-one" as well which is a mess
> 
> Why? That's how DT works and every other platform follows. Either you have:
> 
> "vendor,soc-one", "vendor,family"
> "vendor,soc-two", "vendor,family"
> 
> or
> 
> "vendor,soc-one"
> "vendor,soc-two", "vendor,soc-one"
> 
> The latter is how DT has existed and worked for 20+ years. The former
> is what we allow because for some reason people have such an aversion
> to saying soc2 is compatible with soc1.
> 
> Either one is fine, but the documentation must be clear what the
> constraints are for the dts file. For example, "vendor,soc-two" or
> "vendor,family" alone are not valid. There's plenty of examples to

except that the DT file where it is used is a soc family dtsi.
meson-gx.dtsi is the common DT for gxbb and gxl family. Wouldn't it make sense 
to have the SoC generic compatible alone here ? and override it in the SoC DT if
necessary ? 

> follow. Renesas is one using "vendor,family" extensively.
> 
> > By expressing correctly what the driver is compatible with, "vendor,family"
> > we can dts that makes sense for soc-two as well
> > compatible = "vendor,family", "vendor,soc-two"

Honestly, I don't care which of the 2 solutions we take. The important thing for
me is to eliminate the confusion introduced by the generic-compatible being here
and useless. The driver is indeed generic, but the related compatible is not
matched. This makes no sense.

So either:
* we use "vendor,soc-one", "vendor,family" model: I believe this what I'm trying
 to do here. For this, we need to match the soc generic compatible in the
driver.

* we use the "vendor,soc-two", "vendor,soc-one" model : The driver keeps on
matching the SoC specific compatible. We won't ever match the generic one with
this model, what is the point of keeping it around in DT ? Should we remove it ?

Maybe the later is better, it would end up just being a removal of an
undocumented property from DT.

> 
> Binding docs may live in the kernel, but they are separate from
> drivers. They describe the constraints of the dts files. There's no
> reason to document what the driver wants because I can read the driver
> source. I can't say the same thing about the dts file. The dts may not
> come from the kernel and one dts file is not all possible options for
> a given binding.
> 
> Rob

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

Re: [PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Mon, Oct 23, 2017 at 3:13 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Fri, 2017-10-20 at 14:34 -0500, Rob Herring wrote:
>> On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>> > On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
>> > > On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com>
>> > > wrote:
>> > > > Rob Herring <robh@kernel.org> writes:
>> > > >
>> > > > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> > > > > > The meson secure monitor seems to be compatible with more SoCs than
>> > > > > > initially thought. Let's use the most generic compatible he have in
>> > > > > > DT instead of the gxbb specific one
>> > > > > >
>> > > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> > > > > > ---
>> > > > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4
>> > > > > > ++--
>> > > > > >  drivers/firmware/meson/meson_sm.c                             | 4
>> > > > > > ++--
>> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
>> > > > >
>> > > > > Seems like a pointless, not backwards compatible change to me.
>> > > >
>> > > > I've verified that it's backwards compatible with existing upstream DTs.
>> > >
>> > > Perhaps if you all are documenting only what the driver uses, not what
>> > > the dts can have as Jerome said.
>> > >
>> > > > > end, it's just a string to match on. Who cares what the string is.
>> > > >
>> > > > As platform maintiner, I very much care what the strings are and I want
>> > > > it to be coherent with the platform generic names, and I want the
>> > > > SoC-specific strings to correspond to the actual SoC names.
>> > >
>> > > The most specific compatible should be, absolutely. The fallbacks can
>> > > be anything really. Ideally, they are the compatible string for the
>> > > 1st SoC with "the same" compatible IP. Could be another vendor
>> > > entirely even because mergers happen.
>> >
>> > Then what's your problem with these patches again ?
>>
>> Removal of the SoC specific compatible and breaking compatibility.
>> Kevin says compatibility is not broken, but it obviously it based on
>> the example and the driver change. So that can only mean your dts file
>> doesn't match the example.
>
> I believe it does, but I suppose I have missed something.
>
> Currently the driver only matches on:
> .compatible = "amlogic,meson-gxbb-sm" (drivers/firmware/meson_sm.c)
>
> The only device-tree file using it is
> meson-gx.dtsi: compatible = "amlogic,meson-gx-sm", "amlogic,meson-gxbb-sm";

THIS is not what the binding example shows which is precisely the
problem. Reading the binding doc and driver alone makes it look like
you are breaking compatibility.

> So:
> 1) yes the order is backward (thx for pointing this out)
> 2) by changing the compatible matched by the driver from "amlogic,meson-gxbb-sm"
> to "amlogic,meson-gx-sm", I don't think I am breaking anything. Feel free to
> point out why this is wrong because I don't get it.

You are not breaking compatibility. That's not what I'm saying. If I
had followed the binding doc and wrote my own dts, then compatibility
would have been broken.

>> > I am just asking the driver to match the generic binding instead of the SoC
>> > specific, because we are also using it on other SoC, as explain in the patch
>> > comment. Does not seems that "pointless" to me.
>> >
>> > Right now the driver match only on: vendor,soc-one
>> > in dts, we have compatible = "vendor,family", "vendor,soc-one"
>>
>> That's backwards if I understand this right. It should be most
>> specific first, but that's a separate issue.
>>
>> > but it is compatible with soc-two as well.
>> > to match we would have to put "vendor,soc-one" as well which is a mess
>>
>> Why? That's how DT works and every other platform follows. Either you have:
>>
>> "vendor,soc-one", "vendor,family"
>> "vendor,soc-two", "vendor,family"
>>
>> or
>>
>> "vendor,soc-one"
>> "vendor,soc-two", "vendor,soc-one"
>>
>> The latter is how DT has existed and worked for 20+ years. The former
>> is what we allow because for some reason people have such an aversion
>> to saying soc2 is compatible with soc1.
>>
>> Either one is fine, but the documentation must be clear what the
>> constraints are for the dts file. For example, "vendor,soc-two" or
>> "vendor,family" alone are not valid. There's plenty of examples to
>
> except that the DT file where it is used is a soc family dtsi.
> meson-gx.dtsi is the common DT for gxbb and gxl family. Wouldn't it make sense
> to have the SoC generic compatible alone here ? and override it in the SoC DT if
> necessary ?

That's a detail I don't really care so much about because that's just
dts partitioning. I mainly care how the dtb ends up looking and that
must have an SoC specific compatible.

I will say though, I think that overriding properties like compatible
would not be the best style. Generally, enough things change like
memory address, irq numbers, etc. from SoC to SoC that it's not worth
trying to share dtsi files that way.

>> follow. Renesas is one using "vendor,family" extensively.
>>
>> > By expressing correctly what the driver is compatible with, "vendor,family"
>> > we can dts that makes sense for soc-two as well
>> > compatible = "vendor,family", "vendor,soc-two"
>
> Honestly, I don't care which of the 2 solutions we take. The important thing for
> me is to eliminate the confusion introduced by the generic-compatible being here
> and useless. The driver is indeed generic, but the related compatible is not
> matched. This makes no sense.

The driver change is fine. The dts is fine too it seems, other than
the string ordering. It's the binding doc that is not fine (read the
last paragraph again if you still don't understand why). It needs to
document which of the below you are doing.

> So either:
> * we use "vendor,soc-one", "vendor,family" model: I believe this what I'm trying
>  to do here. For this, we need to match the soc generic compatible in the
> driver.
>
> * we use the "vendor,soc-two", "vendor,soc-one" model : The driver keeps on
> matching the SoC specific compatible. We won't ever match the generic one with
> this model, what is the point of keeping it around in DT ? Should we remove it ?
>
> Maybe the later is better, it would end up just being a removal of an
> undocumented property from DT.
>
>>
>> Binding docs may live in the kernel, but they are separate from
>> drivers. They describe the constraints of the dts files. There's no
>> reason to document what the driver wants because I can read the driver
>> source. I can't say the same thing about the dts file. The dts may not
>> come from the kernel and one dts file is not all possible options for
>> a given binding.

[PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Mon, Oct 23, 2017 at 3:13 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Fri, 2017-10-20 at 14:34 -0500, Rob Herring wrote:
>> On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>> > On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
>> > > On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com>
>> > > wrote:
>> > > > Rob Herring <robh@kernel.org> writes:
>> > > >
>> > > > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> > > > > > The meson secure monitor seems to be compatible with more SoCs than
>> > > > > > initially thought. Let's use the most generic compatible he have in
>> > > > > > DT instead of the gxbb specific one
>> > > > > >
>> > > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> > > > > > ---
>> > > > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4
>> > > > > > ++--
>> > > > > >  drivers/firmware/meson/meson_sm.c                             | 4
>> > > > > > ++--
>> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
>> > > > >
>> > > > > Seems like a pointless, not backwards compatible change to me.
>> > > >
>> > > > I've verified that it's backwards compatible with existing upstream DTs.
>> > >
>> > > Perhaps if you all are documenting only what the driver uses, not what
>> > > the dts can have as Jerome said.
>> > >
>> > > > > end, it's just a string to match on. Who cares what the string is.
>> > > >
>> > > > As platform maintiner, I very much care what the strings are and I want
>> > > > it to be coherent with the platform generic names, and I want the
>> > > > SoC-specific strings to correspond to the actual SoC names.
>> > >
>> > > The most specific compatible should be, absolutely. The fallbacks can
>> > > be anything really. Ideally, they are the compatible string for the
>> > > 1st SoC with "the same" compatible IP. Could be another vendor
>> > > entirely even because mergers happen.
>> >
>> > Then what's your problem with these patches again ?
>>
>> Removal of the SoC specific compatible and breaking compatibility.
>> Kevin says compatibility is not broken, but it obviously it based on
>> the example and the driver change. So that can only mean your dts file
>> doesn't match the example.
>
> I believe it does, but I suppose I have missed something.
>
> Currently the driver only matches on:
> .compatible = "amlogic,meson-gxbb-sm" (drivers/firmware/meson_sm.c)
>
> The only device-tree file using it is
> meson-gx.dtsi: compatible = "amlogic,meson-gx-sm", "amlogic,meson-gxbb-sm";

THIS is not what the binding example shows which is precisely the
problem. Reading the binding doc and driver alone makes it look like
you are breaking compatibility.

> So:
> 1) yes the order is backward (thx for pointing this out)
> 2) by changing the compatible matched by the driver from "amlogic,meson-gxbb-sm"
> to "amlogic,meson-gx-sm", I don't think I am breaking anything. Feel free to
> point out why this is wrong because I don't get it.

You are not breaking compatibility. That's not what I'm saying. If I
had followed the binding doc and wrote my own dts, then compatibility
would have been broken.

>> > I am just asking the driver to match the generic binding instead of the SoC
>> > specific, because we are also using it on other SoC, as explain in the patch
>> > comment. Does not seems that "pointless" to me.
>> >
>> > Right now the driver match only on: vendor,soc-one
>> > in dts, we have compatible = "vendor,family", "vendor,soc-one"
>>
>> That's backwards if I understand this right. It should be most
>> specific first, but that's a separate issue.
>>
>> > but it is compatible with soc-two as well.
>> > to match we would have to put "vendor,soc-one" as well which is a mess
>>
>> Why? That's how DT works and every other platform follows. Either you have:
>>
>> "vendor,soc-one", "vendor,family"
>> "vendor,soc-two", "vendor,family"
>>
>> or
>>
>> "vendor,soc-one"
>> "vendor,soc-two", "vendor,soc-one"
>>
>> The latter is how DT has existed and worked for 20+ years. The former
>> is what we allow because for some reason people have such an aversion
>> to saying soc2 is compatible with soc1.
>>
>> Either one is fine, but the documentation must be clear what the
>> constraints are for the dts file. For example, "vendor,soc-two" or
>> "vendor,family" alone are not valid. There's plenty of examples to
>
> except that the DT file where it is used is a soc family dtsi.
> meson-gx.dtsi is the common DT for gxbb and gxl family. Wouldn't it make sense
> to have the SoC generic compatible alone here ? and override it in the SoC DT if
> necessary ?

That's a detail I don't really care so much about because that's just
dts partitioning. I mainly care how the dtb ends up looking and that
must have an SoC specific compatible.

I will say though, I think that overriding properties like compatible
would not be the best style. Generally, enough things change like
memory address, irq numbers, etc. from SoC to SoC that it's not worth
trying to share dtsi files that way.

>> follow. Renesas is one using "vendor,family" extensively.
>>
>> > By expressing correctly what the driver is compatible with, "vendor,family"
>> > we can dts that makes sense for soc-two as well
>> > compatible = "vendor,family", "vendor,soc-two"
>
> Honestly, I don't care which of the 2 solutions we take. The important thing for
> me is to eliminate the confusion introduced by the generic-compatible being here
> and useless. The driver is indeed generic, but the related compatible is not
> matched. This makes no sense.

The driver change is fine. The dts is fine too it seems, other than
the string ordering. It's the binding doc that is not fine (read the
last paragraph again if you still don't understand why). It needs to
document which of the below you are doing.

> So either:
> * we use "vendor,soc-one", "vendor,family" model: I believe this what I'm trying
>  to do here. For this, we need to match the soc generic compatible in the
> driver.
>
> * we use the "vendor,soc-two", "vendor,soc-one" model : The driver keeps on
> matching the SoC specific compatible. We won't ever match the generic one with
> this model, what is the point of keeping it around in DT ? Should we remove it ?
>
> Maybe the later is better, it would end up just being a removal of an
> undocumented property from DT.
>
>>
>> Binding docs may live in the kernel, but they are separate from
>> drivers. They describe the constraints of the dts files. There's no
>> reason to document what the driver wants because I can read the driver
>> source. I can't say the same thing about the dts file. The dts may not
>> come from the kernel and one dts file is not all possible options for
>> a given binding.

[PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Mon, Oct 23, 2017 at 3:13 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Fri, 2017-10-20 at 14:34 -0500, Rob Herring wrote:
>> On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet@baylibre.com> wrote:
>> > On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
>> > > On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman@baylibre.com>
>> > > wrote:
>> > > > Rob Herring <robh@kernel.org> writes:
>> > > >
>> > > > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> > > > > > The meson secure monitor seems to be compatible with more SoCs than
>> > > > > > initially thought. Let's use the most generic compatible he have in
>> > > > > > DT instead of the gxbb specific one
>> > > > > >
>> > > > > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> > > > > > ---
>> > > > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4
>> > > > > > ++--
>> > > > > >  drivers/firmware/meson/meson_sm.c                             | 4
>> > > > > > ++--
>> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
>> > > > >
>> > > > > Seems like a pointless, not backwards compatible change to me.
>> > > >
>> > > > I've verified that it's backwards compatible with existing upstream DTs.
>> > >
>> > > Perhaps if you all are documenting only what the driver uses, not what
>> > > the dts can have as Jerome said.
>> > >
>> > > > > end, it's just a string to match on. Who cares what the string is.
>> > > >
>> > > > As platform maintiner, I very much care what the strings are and I want
>> > > > it to be coherent with the platform generic names, and I want the
>> > > > SoC-specific strings to correspond to the actual SoC names.
>> > >
>> > > The most specific compatible should be, absolutely. The fallbacks can
>> > > be anything really. Ideally, they are the compatible string for the
>> > > 1st SoC with "the same" compatible IP. Could be another vendor
>> > > entirely even because mergers happen.
>> >
>> > Then what's your problem with these patches again ?
>>
>> Removal of the SoC specific compatible and breaking compatibility.
>> Kevin says compatibility is not broken, but it obviously it based on
>> the example and the driver change. So that can only mean your dts file
>> doesn't match the example.
>
> I believe it does, but I suppose I have missed something.
>
> Currently the driver only matches on:
> .compatible = "amlogic,meson-gxbb-sm" (drivers/firmware/meson_sm.c)
>
> The only device-tree file using it is
> meson-gx.dtsi: compatible = "amlogic,meson-gx-sm", "amlogic,meson-gxbb-sm";

THIS is not what the binding example shows which is precisely the
problem. Reading the binding doc and driver alone makes it look like
you are breaking compatibility.

> So:
> 1) yes the order is backward (thx for pointing this out)
> 2) by changing the compatible matched by the driver from "amlogic,meson-gxbb-sm"
> to "amlogic,meson-gx-sm", I don't think I am breaking anything. Feel free to
> point out why this is wrong because I don't get it.

You are not breaking compatibility. That's not what I'm saying. If I
had followed the binding doc and wrote my own dts, then compatibility
would have been broken.

>> > I am just asking the driver to match the generic binding instead of the SoC
>> > specific, because we are also using it on other SoC, as explain in the patch
>> > comment. Does not seems that "pointless" to me.
>> >
>> > Right now the driver match only on: vendor,soc-one
>> > in dts, we have compatible = "vendor,family", "vendor,soc-one"
>>
>> That's backwards if I understand this right. It should be most
>> specific first, but that's a separate issue.
>>
>> > but it is compatible with soc-two as well.
>> > to match we would have to put "vendor,soc-one" as well which is a mess
>>
>> Why? That's how DT works and every other platform follows. Either you have:
>>
>> "vendor,soc-one", "vendor,family"
>> "vendor,soc-two", "vendor,family"
>>
>> or
>>
>> "vendor,soc-one"
>> "vendor,soc-two", "vendor,soc-one"
>>
>> The latter is how DT has existed and worked for 20+ years. The former
>> is what we allow because for some reason people have such an aversion
>> to saying soc2 is compatible with soc1.
>>
>> Either one is fine, but the documentation must be clear what the
>> constraints are for the dts file. For example, "vendor,soc-two" or
>> "vendor,family" alone are not valid. There's plenty of examples to
>
> except that the DT file where it is used is a soc family dtsi.
> meson-gx.dtsi is the common DT for gxbb and gxl family. Wouldn't it make sense
> to have the SoC generic compatible alone here ? and override it in the SoC DT if
> necessary ?

That's a detail I don't really care so much about because that's just
dts partitioning. I mainly care how the dtb ends up looking and that
must have an SoC specific compatible.

I will say though, I think that overriding properties like compatible
would not be the best style. Generally, enough things change like
memory address, irq numbers, etc. from SoC to SoC that it's not worth
trying to share dtsi files that way.

>> follow. Renesas is one using "vendor,family" extensively.
>>
>> > By expressing correctly what the driver is compatible with, "vendor,family"
>> > we can dts that makes sense for soc-two as well
>> > compatible = "vendor,family", "vendor,soc-two"
>
> Honestly, I don't care which of the 2 solutions we take. The important thing for
> me is to eliminate the confusion introduced by the generic-compatible being here
> and useless. The driver is indeed generic, but the related compatible is not
> matched. This makes no sense.

The driver change is fine. The dts is fine too it seems, other than
the string ordering. It's the binding doc that is not fine (read the
last paragraph again if you still don't understand why). It needs to
document which of the below you are doing.

> So either:
> * we use "vendor,soc-one", "vendor,family" model: I believe this what I'm trying
>  to do here. For this, we need to match the soc generic compatible in the
> driver.
>
> * we use the "vendor,soc-two", "vendor,soc-one" model : The driver keeps on
> matching the SoC specific compatible. We won't ever match the generic one with
> this model, what is the point of keeping it around in DT ? Should we remove it ?
>
> Maybe the later is better, it would end up just being a removal of an
> undocumented property from DT.
>
>>
>> Binding docs may live in the kernel, but they are separate from
>> drivers. They describe the constraints of the dts files. There's no
>> reason to document what the driver wants because I can read the driver
>> source. I can't say the same thing about the dts file. The dts may not
>> come from the kernel and one dts file is not all possible options for
>> a given binding.

Re: [PATCH] firmware: meson-sm: use generic compatible

[Rob Herring]

On Mon, Oct 23, 2017 at 3:13 AM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
> On Fri, 2017-10-20 at 14:34 -0500, Rob Herring wrote:
>> On Fri, Oct 20, 2017 at 3:30 AM, Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>> > On Thu, 2017-10-19 at 16:18 -0500, Rob Herring wrote:
>> > > On Thu, Oct 19, 2017 at 5:25 AM, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> > > wrote:
>> > > > Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:
>> > > >
>> > > > > On Thu, Oct 12, 2017 at 03:47:43PM +0200, Jerome Brunet wrote:
>> > > > > > The meson secure monitor seems to be compatible with more SoCs than
>> > > > > > initially thought. Let's use the most generic compatible he have in
>> > > > > > DT instead of the gxbb specific one
>> > > > > >
>> > > > > > Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>> > > > > > ---
>> > > > > >  Documentation/devicetree/bindings/firmware/meson/meson_sm.txt | 4
>> > > > > > ++--
>> > > > > >  drivers/firmware/meson/meson_sm.c                             | 4
>> > > > > > ++--
>> > > > > >  2 files changed, 4 insertions(+), 4 deletions(-)
>> > > > >
>> > > > > Seems like a pointless, not backwards compatible change to me.
>> > > >
>> > > > I've verified that it's backwards compatible with existing upstream DTs.
>> > >
>> > > Perhaps if you all are documenting only what the driver uses, not what
>> > > the dts can have as Jerome said.
>> > >
>> > > > > end, it's just a string to match on. Who cares what the string is.
>> > > >
>> > > > As platform maintiner, I very much care what the strings are and I want
>> > > > it to be coherent with the platform generic names, and I want the
>> > > > SoC-specific strings to correspond to the actual SoC names.
>> > >
>> > > The most specific compatible should be, absolutely. The fallbacks can
>> > > be anything really. Ideally, they are the compatible string for the
>> > > 1st SoC with "the same" compatible IP. Could be another vendor
>> > > entirely even because mergers happen.
>> >
>> > Then what's your problem with these patches again ?
>>
>> Removal of the SoC specific compatible and breaking compatibility.
>> Kevin says compatibility is not broken, but it obviously it based on
>> the example and the driver change. So that can only mean your dts file
>> doesn't match the example.
>
> I believe it does, but I suppose I have missed something.
>
> Currently the driver only matches on:
> .compatible = "amlogic,meson-gxbb-sm" (drivers/firmware/meson_sm.c)
>
> The only device-tree file using it is
> meson-gx.dtsi: compatible = "amlogic,meson-gx-sm", "amlogic,meson-gxbb-sm";

THIS is not what the binding example shows which is precisely the
problem. Reading the binding doc and driver alone makes it look like
you are breaking compatibility.

> So:
> 1) yes the order is backward (thx for pointing this out)
> 2) by changing the compatible matched by the driver from "amlogic,meson-gxbb-sm"
> to "amlogic,meson-gx-sm", I don't think I am breaking anything. Feel free to
> point out why this is wrong because I don't get it.

You are not breaking compatibility. That's not what I'm saying. If I
had followed the binding doc and wrote my own dts, then compatibility
would have been broken.

>> > I am just asking the driver to match the generic binding instead of the SoC
>> > specific, because we are also using it on other SoC, as explain in the patch
>> > comment. Does not seems that "pointless" to me.
>> >
>> > Right now the driver match only on: vendor,soc-one
>> > in dts, we have compatible = "vendor,family", "vendor,soc-one"
>>
>> That's backwards if I understand this right. It should be most
>> specific first, but that's a separate issue.
>>
>> > but it is compatible with soc-two as well.
>> > to match we would have to put "vendor,soc-one" as well which is a mess
>>
>> Why? That's how DT works and every other platform follows. Either you have:
>>
>> "vendor,soc-one", "vendor,family"
>> "vendor,soc-two", "vendor,family"
>>
>> or
>>
>> "vendor,soc-one"
>> "vendor,soc-two", "vendor,soc-one"
>>
>> The latter is how DT has existed and worked for 20+ years. The former
>> is what we allow because for some reason people have such an aversion
>> to saying soc2 is compatible with soc1.
>>
>> Either one is fine, but the documentation must be clear what the
>> constraints are for the dts file. For example, "vendor,soc-two" or
>> "vendor,family" alone are not valid. There's plenty of examples to
>
> except that the DT file where it is used is a soc family dtsi.
> meson-gx.dtsi is the common DT for gxbb and gxl family. Wouldn't it make sense
> to have the SoC generic compatible alone here ? and override it in the SoC DT if
> necessary ?

That's a detail I don't really care so much about because that's just
dts partitioning. I mainly care how the dtb ends up looking and that
must have an SoC specific compatible.

I will say though, I think that overriding properties like compatible
would not be the best style. Generally, enough things change like
memory address, irq numbers, etc. from SoC to SoC that it's not worth
trying to share dtsi files that way.

>> follow. Renesas is one using "vendor,family" extensively.
>>
>> > By expressing correctly what the driver is compatible with, "vendor,family"
>> > we can dts that makes sense for soc-two as well
>> > compatible = "vendor,family", "vendor,soc-two"
>
> Honestly, I don't care which of the 2 solutions we take. The important thing for
> me is to eliminate the confusion introduced by the generic-compatible being here
> and useless. The driver is indeed generic, but the related compatible is not
> matched. This makes no sense.

The driver change is fine. The dts is fine too it seems, other than
the string ordering. It's the binding doc that is not fine (read the
last paragraph again if you still don't understand why). It needs to
document which of the below you are doing.

> So either:
> * we use "vendor,soc-one", "vendor,family" model: I believe this what I'm trying
>  to do here. For this, we need to match the soc generic compatible in the
> driver.
>
> * we use the "vendor,soc-two", "vendor,soc-one" model : The driver keeps on
> matching the SoC specific compatible. We won't ever match the generic one with
> this model, what is the point of keeping it around in DT ? Should we remove it ?
>
> Maybe the later is better, it would end up just being a removal of an
> undocumented property from DT.
>
>>
>> Binding docs may live in the kernel, but they are separate from
>> drivers. They describe the constraints of the dts files. There's no
>> reason to document what the driver wants because I can read the driver
>> source. I can't say the same thing about the dts file. The dts may not
>> come from the kernel and one dts file is not all possible options for
>> a given binding.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html