linux-kernel - Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function defines

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ad1d6964-f553-3a22-65fd-58bc5a166f80@axentia.se>
Date:   Thu, 17 Sep 2020 14:37:30 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Roger Quadros <rogerq@...com>, Nishanth Menon <nm@...com>
Cc:     t-kristo@...com, robh+dt@...nel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        nsekhar@...com, kishon@...com
Subject: Re: [PATCH v3 1/6] dt-bindings: mux-j7200-wiz: Add lane function
 defines

Hi!

On 2020-09-17 14:00, Roger Quadros wrote:
> Hi Peter & Nishanth,
> 
> On 16/09/2020 18:45, Nishanth Menon wrote:
>> On 06:52-20200916, Peter Rosin wrote:
>>> Hi,
>>>
>>> Sorry for the delay.
>>>
>>> On 2020-09-15 13:20, Roger Quadros wrote:
>>>> Each SERDES lane mux can select upto 4 different IPs.
>>>> There are 4 lanes in each J7200 SERDES. Define all
>>>> the possible functions in this file.
>>>>
>>>> Cc: Peter Rosin <peda@...ntia.se>
>>>> Signed-off-by: Roger Quadros <rogerq@...com>
>>>> ---
>>>>   include/dt-bindings/mux/mux-j7200-wiz.h | 29 +++++++++++++++++++++++++
>>>>   1 file changed, 29 insertions(+)
>>>>   create mode 100644 include/dt-bindings/mux/mux-j7200-wiz.h
>>>>
>>>> diff --git a/include/dt-bindings/mux/mux-j7200-wiz.h b/include/dt-bindings/mux/mux-j7200-wiz.h
>>>> new file mode 100644
>>>> index 000000000000..b091b1185a36
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/mux/mux-j7200-wiz.h
>>>> @@ -0,0 +1,29 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>>> +/*
>>>> + * This header provides constants for J7200 WIZ.
>>>> + */
>>>> +
>>>> +#ifndef _DT_BINDINGS_J7200_WIZ
>>>> +#define _DT_BINDINGS_J7200_WIZ
>>>> +
>>>> +#define SERDES0_LANE0_QSGMII_LANE3	0x0
>>>> +#define SERDES0_LANE0_PCIE1_LANE0	0x1
>>>> +#define SERDES0_LANE0_IP3_UNUSED	0x2
>>>> +#define SERDES0_LANE0_IP4_UNUSED	0x3
>>>> +
>>>> +#define SERDES0_LANE1_QSGMII_LANE4	0x0
>>>> +#define SERDES0_LANE1_PCIE1_LANE1	0x1
>>>> +#define SERDES0_LANE1_IP3_UNUSED	0x2
>>>> +#define SERDES0_LANE1_IP4_UNUSED	0x3
>>>> +
>>>> +#define SERDES0_LANE2_QSGMII_LANE1	0x0
>>>> +#define SERDES0_LANE2_PCIE1_LANE2	0x1
>>>> +#define SERDES0_LANE2_IP3_UNUSED	0x2
>>>> +#define SERDES0_LANE2_IP4_UNUSED	0x3
>>>> +
>>>> +#define SERDES0_LANE3_QSGMII_LANE2	0x0
>>>> +#define SERDES0_LANE3_PCIE1_LANE3	0x1
>>>> +#define SERDES0_LANE3_USB		0x2
>>>> +#define SERDES0_LANE3_IP4_UNUSED	0x3
>>>> +
>>>> +#endif /* _DT_BINDINGS_J7200_WIZ */
>>>
>>> Should not the defines start with J7200_WIZ? SERDES0 seems like a too
>>> generic prefix, at least to me.
>>
>> Thanks, good point. I am not sure if WIZ should even be used.. It is
>> a TI internal prefix for various serdes solutions, but I agree that
>> SERDES0 is too generic a terminology. That said, we should cleanup
>> include/dt-bindings/mux/mux-j721e-wiz.h as well, prior to introducing
>> j7200 changes.
>>
> 
> I'm planning to put all TI SERDES definitions in one header file "ti-serdes-mux.h"
> and add SOC specific prefixes to the macros.
> 
> This will mean some churn in the existing DT files. (only 2 so far)
> 
> Are you guys OK if I do the change in one patch to avoid a broken build in between.
> You guys can then decide whose tree it goes through.
> 
> The new SoC addition will be separate of course.

We should get these changes done before 5.9 is released.

Not breaking the build for each intermediate step is always a priority.
Also, renaming mux-j721e-wiz.h to ti-serdes-mux.h and renaming the macros
could be seen as orthogonal, and it is certainly possible to do that
as two patches without breaking the build in between. It would just need
changes on both sides of the interface in both patches. But I wouldn't
worry about separating this into two patches, just do a rename patch and
be done with it. Then follow up with additions for j7200.

However, now that we are renaming things anyway, do we really need "mux"
in the name of the file itself?
I personally find .../dt-dbindings/mux/ti-serdes.h descriptive enough.

Cheers,
Peter