[v2] ASoC: samsung: Mark unused Odroid compatibles as deprecated
diff mbox

Message ID 20180318153512.8343-1-krzk@kernel.org
State Accepted
Commit 3b84574d95c020f01410ff2ca0a38dd8fd7b644b
Headers show

Commit Message

Krzysztof Kozlowski March 18, 2018, 3:35 p.m. UTC
Compatible for XU4 audio is not being used.  Instead the board uses the
same compatible as XU3.  The devices are now just compatible so they
should use the same value.  Mark "hardkernel,odroid-xu4-audio" as being
deprecated so in this future could be removed to limit useless
properties.

Additionally add older compatibles appearing in the bindings.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Changes since v1:
1. Correct typo XU4->XU3 for one deprecated compatible.
---
 Documentation/devicetree/bindings/sound/samsung,odroid.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Sylwester Nawrocki March 19, 2018, 10:29 a.m. UTC | #1
On 03/18/2018 04:35 PM, Krzysztof Kozlowski wrote:
> Compatible for XU4 audio is not being used.  Instead the board uses the
> same compatible as XU3.  The devices are now just compatible so they
> should use the same value.  Mark "hardkernel,odroid-xu4-audio" as being
> deprecated so in this future could be removed to limit useless
> properties.

It doesn't feel right to obsolete the "hardkernel,odroid-xu4-audio"
compatible, there is significant difference between XU3 and XU4 - there
is no audio CODEC on XU4, this board only supports audio over HDMI interface.
XU4 could be compatible with XU3, but not the other way around.
It just happens we have other DT properties that help to handle such HW 
design difference.

Moreover, only XU4 is still in production and should be in few more years [1], 
others are obsoleted now.

So I think we should keep at least these 2 compatible strings:

- "hardkernel,odroid-xu3-audio" - for boards with audio CODEC,
- "hardkernel,odroid-xu4-audio" - for boards without audio CODEC, 
   supporting only HDMI interface.

--
Thanks,
Sylwester

[1] https://com.odroid.com/sigong/blog/blog_list.php
Krzysztof Kozlowski March 19, 2018, 10:56 a.m. UTC | #2
On Mon, Mar 19, 2018 at 11:29 AM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> On 03/18/2018 04:35 PM, Krzysztof Kozlowski wrote:
>> Compatible for XU4 audio is not being used.  Instead the board uses the
>> same compatible as XU3.  The devices are now just compatible so they
>> should use the same value.  Mark "hardkernel,odroid-xu4-audio" as being
>> deprecated so in this future could be removed to limit useless
>> properties.
>
> It doesn't feel right to obsolete the "hardkernel,odroid-xu4-audio"
> compatible, there is significant difference between XU3 and XU4 - there
> is no audio CODEC on XU4, this board only supports audio over HDMI interface.
> XU4 could be compatible with XU3, but not the other way around.
> It just happens we have other DT properties that help to handle such HW
> design difference.

The compatible does not describe physical differences. It does not
mean that devices are the same. In this case they are just coming from
the same family and they operate the same, from the bindings
perspective.

The XU4 binding is not being used. Adding a compatible which is not
used in the moment of adding is a proof that this compatible is not
needed. It is just a duplicate. There is no point of adding
duplicates.

>
> Moreover, only XU4 is still in production and should be in few more years [1],
> others are obsoleted now.

It is not a problem. Whether device is manufactured or not, does not
reflect what bindings we are using. Deprecated XU4 compatible does not
mean that XU4 itself is deprecated. Just this compatible should not be
used for new DTS.

>
> So I think we should keep at least these 2 compatible strings:
>
> - "hardkernel,odroid-xu3-audio" - for boards with audio CODEC,
> - "hardkernel,odroid-xu4-audio" - for boards without audio CODEC,
>    supporting only HDMI interface.

Yeah, and then we inflate this list into X, X2, U3, HC1 and all others
which are the same. And then we should add XU3-lite (it is different
device). This goes to some nonsense. Compatible is not for each device
but for family even though there are differences between specific
devices.

Best regards,
Krzysztof
Sylwester Nawrocki March 19, 2018, 3:14 p.m. UTC | #3
On 03/19/2018 11:56 AM, Krzysztof Kozlowski wrote:
> On Mon, Mar 19, 2018 at 11:29 AM, Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
>> On 03/18/2018 04:35 PM, Krzysztof Kozlowski wrote:
>>> Compatible for XU4 audio is not being used.  Instead the board uses the
>>> same compatible as XU3.  The devices are now just compatible so they
>>> should use the same value.  Mark "hardkernel,odroid-xu4-audio" as being
>>> deprecated so in this future could be removed to limit useless
>>> properties.
>>
>> It doesn't feel right to obsolete the "hardkernel,odroid-xu4-audio"
>> compatible, there is significant difference between XU3 and XU4 - there
>> is no audio CODEC on XU4, this board only supports audio over HDMI interface.
>> XU4 could be compatible with XU3, but not the other way around.
>> It just happens we have other DT properties that help to handle such HW
>> design difference.
> 
> The compatible does not describe physical differences. It does not
> mean that devices are the same. In this case they are just coming from
> the same family and they operate the same, from the bindings
> perspective.

From the ePAPR 'compatible' string definition you cited, the compatible 
string is supposed to indicate programming model of a device, for the purpose 
of matching a driver. I thought the programming model refers to the driver's
SW interfaces used to control the hardware, rather than only to a particular
DT binding design. And XU4 is not compatible with XU3 from device programming
perspective.

> The XU4 binding is not being used. Adding a compatible which is not
> used in the moment of adding is a proof that this compatible is not
> needed. It is just a duplicate. There is no point of adding
> duplicates.

I disagree it is just an unnecessary duplicate, I think dts for XU4 could
fixed instead of dropping that compatible from the binding.
>> Moreover, only XU4 is still in production and should be in few more years [1],
>> others are obsoleted now.
> 
> It is not a problem. Whether device is manufactured or not, does not
> reflect what bindings we are using. Deprecated XU4 compatible does not
> mean that XU4 itself is deprecated. Just this compatible should not be
> used for new DTS.

>> So I think we should keep at least these 2 compatible strings:
>>
>> - "hardkernel,odroid-xu3-audio" - for boards with audio CODEC,
>> - "hardkernel,odroid-xu4-audio" - for boards without audio CODEC,
>>    supporting only HDMI interface.
> 
> Yeah, and then we inflate this list into X, X2, U3, HC1 and all others
> which are the same. And then we should add XU3-lite (it is different
> device). This goes to some nonsense. Compatible is not for each device
> but for family even though there are differences between specific
> devices.

You are not listening, I refer only to major audio subsystem differences.
It would have been:

 - "hardkernel,odroid-xu3-audio" for: U2, U3, X, X2, XU, XU3, XU3-Lite
 - "hardkernel,odroid-xu4-audio" for: XU4

But if you insist on only one compatible I'm not going to argue further,
you will be responsible for this. :)
Sylwester Nawrocki March 19, 2018, 3:16 p.m. UTC | #4
On 03/18/2018 04:35 PM, Krzysztof Kozlowski wrote:
> diff --git a/Documentation/devicetree/bindings/sound/samsung,odroid.txt b/Documentation/devicetree/bindings/sound/samsung,odroid.txt
> index f35a2694eb04..e9da2200e173 100644
> --- a/Documentation/devicetree/bindings/sound/samsung,odroid.txt
> +++ b/Documentation/devicetree/bindings/sound/samsung,odroid.txt
> @@ -3,7 +3,9 @@ Samsung Exynos Odroid XU3/XU4 audio complex with MAX98090 codec
>  Required properties:
>  
>   - compatible - "hardkernel,odroid-xu3-audio" - for Odroid XU3 board,
> -		"hardkernel,odroid-xu4-audio" - for Odroid XU4 board
> +		"hardkernel,odroid-xu4-audio" - for Odroid XU4 board (deprecated),
> +		"samsung,odroid-xu3-audio" - for Odroid XU3 board (deprecated),
> +		"samsung,odroid-xu4-audio" - for Odroid XU4 board (deprecated)

I think we should make it clear which compatible applies to which board.
Krzysztof Kozlowski March 20, 2018, 7:11 a.m. UTC | #5
On Mon, Mar 19, 2018 at 4:14 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> On 03/19/2018 11:56 AM, Krzysztof Kozlowski wrote:
>> On Mon, Mar 19, 2018 at 11:29 AM, Sylwester Nawrocki
>> <s.nawrocki@samsung.com> wrote:
>>> On 03/18/2018 04:35 PM, Krzysztof Kozlowski wrote:
>>>> Compatible for XU4 audio is not being used.  Instead the board uses the
>>>> same compatible as XU3.  The devices are now just compatible so they
>>>> should use the same value.  Mark "hardkernel,odroid-xu4-audio" as being
>>>> deprecated so in this future could be removed to limit useless
>>>> properties.
>>>
>>> It doesn't feel right to obsolete the "hardkernel,odroid-xu4-audio"
>>> compatible, there is significant difference between XU3 and XU4 - there
>>> is no audio CODEC on XU4, this board only supports audio over HDMI interface.
>>> XU4 could be compatible with XU3, but not the other way around.
>>> It just happens we have other DT properties that help to handle such HW
>>> design difference.
>>
>> The compatible does not describe physical differences. It does not
>> mean that devices are the same. In this case they are just coming from
>> the same family and they operate the same, from the bindings
>> perspective.
>
> From the ePAPR 'compatible' string definition you cited, the compatible
> string is supposed to indicate programming model of a device, for the purpose
> of matching a driver.

Yes, you're correct, it refers to programming model. Although later
you will find second explanation (chapter 4): "The compatible property
of a device node describes the specific binding (or bindings) to which
the node complies.".

> I thought the programming model refers to the driver's
> SW interfaces used to control the hardware, rather than only to a particular
> DT binding design. And XU4 is not compatible with XU3 from device programming
> perspective.

The programming models of XU4 and XU3 audio components, to which we
refer now and which are implemented/used, are the same. I mean not the
same in general, but how we use them. The used subset of each is the
same. Therefore the binding and the driver do not distinguish any
differences (like codec).

>> The XU4 binding is not being used. Adding a compatible which is not
>> used in the moment of adding is a proof that this compatible is not
>> needed. It is just a duplicate. There is no point of adding
>> duplicates.
>
> I disagree it is just an unnecessary duplicate, I think dts for XU4 could
> fixed instead of dropping that compatible from the binding.

You know, there is nothing to fix - changing the compatible to XU4
will not change anything. The executed code will be exactly the
same...

>>> Moreover, only XU4 is still in production and should be in few more years [1],
>>> others are obsoleted now.
>>
>> It is not a problem. Whether device is manufactured or not, does not
>> reflect what bindings we are using. Deprecated XU4 compatible does not
>> mean that XU4 itself is deprecated. Just this compatible should not be
>> used for new DTS.
>
>>> So I think we should keep at least these 2 compatible strings:
>>>
>>> - "hardkernel,odroid-xu3-audio" - for boards with audio CODEC,
>>> - "hardkernel,odroid-xu4-audio" - for boards without audio CODEC,
>>>    supporting only HDMI interface.
>>
>> Yeah, and then we inflate this list into X, X2, U3, HC1 and all others
>> which are the same. And then we should add XU3-lite (it is different
>> device). This goes to some nonsense. Compatible is not for each device
>> but for family even though there are differences between specific
>> devices.
>
> You are not listening, I refer only to major audio subsystem differences.
> It would have been:
>
>  - "hardkernel,odroid-xu3-audio" for: U2, U3, X, X2, XU, XU3, XU3-Lite
>  - "hardkernel,odroid-xu4-audio" for: XU4
>
> But if you insist on only one compatible I'm not going to argue further,
> you will be responsible for this. :)

Ah, I read too fast and missed that point. Actually that is nice
consensus in this case.

Best regards,
Krzysztof
Sylwester Nawrocki March 20, 2018, 6:08 p.m. UTC | #6
On 03/20/2018 08:11 AM, Krzysztof Kozlowski wrote:
> On Mon, Mar 19, 2018 at 4:14 PM, Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
>> On 03/19/2018 11:56 AM, Krzysztof Kozlowski wrote:
>>> On Mon, Mar 19, 2018 at 11:29 AM, Sylwester Nawrocki
>>> <s.nawrocki@samsung.com> wrote:
>>>> On 03/18/2018 04:35 PM, Krzysztof Kozlowski wrote:

>>> The compatible does not describe physical differences. It does not
>>> mean that devices are the same. In this case they are just coming from
>>> the same family and they operate the same, from the bindings
>>> perspective.
>>
>> From the ePAPR 'compatible' string definition you cited, the compatible
>> string is supposed to indicate programming model of a device, for the purpose
>> of matching a driver.
> 
> Yes, you're correct, it refers to programming model. Although later
> you will find second explanation (chapter 4): "The compatible property
> of a device node describes the specific binding (or bindings) to which
> the node complies.".

Yes, I'm aware of that.
>> I thought the programming model refers to the driver's
>> SW interfaces used to control the hardware, rather than only to a particular
>> DT binding design. And XU4 is not compatible with XU3 from device programming
>> perspective.
> 
> The programming models of XU4 and XU3 audio components, to which we
> refer now and which are implemented/used, are the same. I mean not the
> same in general, but how we use them. The used subset of each is the
> same. Therefore the binding and the driver do not distinguish any
> differences (like codec).
 
Actually the driver has to determine the number of codecs, as one of them 
requires additional configuration steps. But this is currently being taken
care of by looking at number of entries in one of the properties.

>>> The XU4 binding is not being used. Adding a compatible which is not
>>> used in the moment of adding is a proof that this compatible is not
>>> needed. It is just a duplicate. There is no point of adding
>>> duplicates.
>>
>> I disagree it is just an unnecessary duplicate, I think dts for XU4 could
>> fixed instead of dropping that compatible from the binding.
> 
> You know, there is nothing to fix - changing the compatible to XU4
> will not change anything. The executed code will be exactly the
> same...

Right, there is really no need now for another compatible as the 
differences across boards are handled through additional properties. 

>>>> So I think we should keep at least these 2 compatible strings:
>>>>
>>>> - "hardkernel,odroid-xu3-audio" - for boards with audio CODEC,
>>>> - "hardkernel,odroid-xu4-audio" - for boards without audio CODEC,
>>>>    supporting only HDMI interface.
>>>
>>> Yeah, and then we inflate this list into X, X2, U3, HC1 and all others
>>> which are the same. And then we should add XU3-lite (it is different
>>> device). This goes to some nonsense. Compatible is not for each device
>>> but for family even though there are differences between specific
>>> devices.
>>
>> You are not listening, I refer only to major audio subsystem differences.
>> It would have been:
>>
>>  - "hardkernel,odroid-xu3-audio" for: U2, U3, X, X2, XU, XU3, XU3-Lite
>>  - "hardkernel,odroid-xu4-audio" for: XU4
>>
>> But if you insist on only one compatible I'm not going to argue further,
>> you will be responsible for this. :)
> 
> Ah, I read too fast and missed that point. Actually that is nice
> consensus in this case.

As Mark said, it is not essential what we decide here, my preference was 
to keep these 2 compatibles. I might got confused a bit and did not consider
a more feature complete "hardkernel,odroid-xu3-audio" binding being applied 
to a XU4 device supporting only a subset of functionality.

Patch
diff mbox

diff --git a/Documentation/devicetree/bindings/sound/samsung,odroid.txt b/Documentation/devicetree/bindings/sound/samsung,odroid.txt
index f35a2694eb04..e9da2200e173 100644
--- a/Documentation/devicetree/bindings/sound/samsung,odroid.txt
+++ b/Documentation/devicetree/bindings/sound/samsung,odroid.txt
@@ -3,7 +3,9 @@  Samsung Exynos Odroid XU3/XU4 audio complex with MAX98090 codec
 Required properties:
 
  - compatible - "hardkernel,odroid-xu3-audio" - for Odroid XU3 board,
-		"hardkernel,odroid-xu4-audio" - for Odroid XU4 board
+		"hardkernel,odroid-xu4-audio" - for Odroid XU4 board (deprecated),
+		"samsung,odroid-xu3-audio" - for Odroid XU3 board (deprecated),
+		"samsung,odroid-xu4-audio" - for Odroid XU4 board (deprecated)
  - model - the user-visible name of this sound complex
  - clocks - should contain entries matching clock names in the clock-names
     property