Hi, On Fri, 2021-11-19 at 11:31 +0100, Matthias Brugger wrote: > > > 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@xxxxxxxxxxxx/__;!!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? > Okay, I have sent new patch set based on your suggestion, Basic Part: https://lore.kernel.org/all/20211122123222.8016-1-sam.shih@xxxxxxxxxxxx/ Pinctrl: https://lore.kernel.org/all/20211122123552.8218-1-sam.shih@xxxxxxxxxxxx/ Please take a look at those patches when you are free. Regards, Sam > Regards, > Matthias >
