lkml.org 
[lkml]   [2021]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v7 2/3] arm64: dts: mediatek: add basic mt7986a support


On 18/11/2021 04:48, Sam Shih wrote:
> Hi
>
> On Tue, 2021-11-16 at 12:18 +0100, Matthias Brugger wrote:
>>
>> On 16/11/2021 02:39, Sam Shih wrote:
>>> Hi,
>>>
>>> On Mon, 2021-11-15 at 17:27 +0100, Matthias Brugger wrote:
>>>> Hi,
>>>>
>>>> On 18/10/2021 13:40, Sam Shih wrote:
>>>>> Add basic chip support for Mediatek mt7986a, include
>>>>> basic uart nodes, rng node and watchdog node.
>>>>>
>>>>> Add cpu node, timer node, gic node, psci and reserved-memory
>>>>> node
>>>>> for ARM Trusted Firmware.
>>>>>
>>>>
>>>> What is the exact difference between mt7986a and mt7986b? Right
>>>> now,
>>>> it's only
>>>> the compatible, for that it makes no sense to split them.
>>>>
>>>
>>> The difference between mt7986a and mt7986b is pinout which
>>> described
>>> in our pinctrl patch series
>>>
> https://urldefense.com/v3/__https://lore.kernel.org/all/20211022124036.5291-3-sam.shih@mediatek.com/__;!!CTRNKA9wMg0ARbw!0kseU8x1KnHHXDErh6Yj6MKqecufPEfGyeumtTBism47e99UFO2Gs-HfWjL1_jUv$
>>>
>>>
>>> You are right, in this "basic SoC support" patch series, only show
>>> compatible differences
>>>
>>>> It would be good to see what the exact differences are, so that
>>>> we
>>>> can see if it
>>>> makes sense to have one of the alternatives:
>>>> 1) use a common mt7986.dtsi which get included by
>>>> mt7986[a,b].dtsi
>>>> 2) Use on mt7986.dtsi and only add one mt7986a.dtsi or
>>>> mt7986b.dtsi
>>>> which has
>>>> add-ons.
>>>>
>>>
>>> In this case, can we use solution (1) to create a generic
>>> mt7986.dtsi
>>> in this patch series, and add mt7986[a,b].dtsi to the dts part of
>>> the
>>> pinctrl patch series to separate the difference nodes?
>>>
>>
>> If the only difference is the GPIO controller then why not go with
>> solution 2.
>> Create a mt7986.dtsi which holds e.g. the node for pincontroller
>> mt7986a and
>> then create a mt7986b.dtsi that just changes compatible and gpio-
>> ranges:
>>
>> &pio {
>> compatible = "mediatek,mt7986b-pinctrl";
>> gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
>> }
>>
>> What do you think?
>
> Ok,
>
> For this basic patch series DTS, I will send the next version:
> - Use "mt7986.dtsi" instead of "mt7986[a,b].dtsi",
> And make"mt7986.dtsi" get included by "mt7986[a,b]-rfb.dts"
> (No dedicated uart1/uart2 pinout for mt7986b-rfb, status of dts node
> shoud be set to "disabled")
>
>
> For the pinctrl patch series DTS, I will send th next version:
> - Add "mt7986b.dtsi" according to your suggestion,
> the new include
> chain will be:
> mt7986a-rfb.dts <-- mt7986.dtsi (mt7986a pinctrl)
>
> mt7986b-rfb.dts <-- mt7986b.dtsi (mt7986b pinctrl) <-- mt7986.dtsi
> (mt7986a pinctrl)
>
> Do you agree above proposal?
>

I mean something like this:
mt7986a.dtsi:
pio: pinctrl@1001f000 {
compatible = "mediatek,mt7986a-pinctrl";
reg = <0 0x1001f000 0 0x1000>,
<0 0x11c30000 0 0x1000>,
<0 0x11c40000 0 0x1000>,
<0 0x11e20000 0 0x1000>,
<0 0x11e30000 0 0x1000>,
<0 0x11f00000 0 0x1000>,
<0 0x11f10000 0 0x1000>,
<0 0x1000b000 0 0x1000>;
reg-names = "gpio", "iocfg_rt", "iocfg_rb", "iocfg_lt",
"iocfg_lb", "iocfg_tr", "iocfg_tl", "eint";
gpio-controller;
#gpio-cells = <2>;
gpio-ranges = <&pio 0 0 100>;
interrupt-controller;
interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
interrupt-parent = <&gic>;
#interrupt-cells = <2>;
};

mt7986b.dtsi:
#include "mt7986a.dtsi"

&pio {
compatible = "mediatek,mt7986b-pinctrl";
gpio-ranges = <&pio 0 0 41>, <&pio 66 66 35>;
}

mt7986b-rfb.dts:
#include "mt7986b.dtsi"

&pio {
uart1_pins: uart1-pins {
mux { [...]


mt7986a-rfb.dts:
#include "mt7986a.dtsi"

&pio {
uart1_pins: uart1-pins {
mux { [...]


Makes sense?

Regards,
Matthias

\
 
 \ /
  Last update: 2021-11-19 11:31    [W:0.052 / U:0.624 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site