Dear Krzysztof,
Thanks for your comments~
On Tue, 2022-10-18 at 08:51 -0400, Krzysztof Kozlowski wrote:
> On 18/10/2022 02:37, Biao Huang wrote:
> > Dear Krzysztof,
> > Thanks for your comments!
> >
> > On Mon, 2022-10-17 at 22:01 -0400, Krzysztof Kozlowski wrote:
> > > On 17/10/2022 05:58, Biao Huang wrote:
> > > > Add Ethernet controller node for mt8195.
> > > >
> > > > Signed-off-by: Biao Huang <biao.huang@xxxxxxxxxxxx>
> > > > ---
> > > > arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 88
> > > > ++++++++++++++++++++
> > > > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 87
> > > > +++++++++++++++++++
> > > > 2 files changed, 175 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > > > b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > > > index 4fbd99eb496a..02e04f82a4ae 100644
> > > > --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > > > +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > > > @@ -258,6 +258,72 @@ &mt6359_vsram_others_ldo_reg {
> > > > };
> > > >
> > > > &pio {
> > > > + eth_default: eth_default {
> > >
> > > No underscores in node names. Please also be sure your patch does
> > > not
> > > bring new warnings with `dtbs_check` (lack of suffix above could
> > > mean
> > > it
> > > brings...)
> >
> > OK, I'll fix the underscores issue in next send.
> > As to "lack of suffix" issue, do you mean I should write it like:
> > eth-default: eth-default@0 {
>
> I don't know whether you should have here suffix or not - please
> check
> your bindings. Several pinctrl bindings require suffixes (or
> prefixes),
> thus I asked.
>
> BTW, In the label you must use underscore.
OK, I'll check the pinctrl-mt8195.yaml, and modify the eth related
node.
>
> > ...
> > }
> > If yes, other nodes in current file don't have such suffix.
> > e.g.
> > gpio_keys_pins: gpio-keys-pins
> >
> > Should I keep unified style with other nodes?
>
> Check what bindings are requiring.
OK.
>
> > >
> > > > + txd_pins {
>
> (...)
>
> > > > +
> > > > + eth: ethernet@11021000 {
> > > > + compatible = "mediatek,mt8195-gmac",
> > > > "snps,dwmac-5.10a";
> > > > + reg = <0 0x11021000 0 0x4000>;
> > > > + interrupts = <GIC_SPI 716
> > > > IRQ_TYPE_LEVEL_HIGH
> > > > 0>;
> > > > + interrupt-names = "macirq";
> > > > + mac-address = [00 55 7b b5 7d f7];
> > >
> > > How is this property of a SoC? Are you saying now that all MT8195
> > > SoCs
> > > have the same MAC address?
> >
> > The mac-address here is taken as a default mac address in eth
> > driver
> > rather than a randome one.
> > Actually, there will be a tool to customize eth mac address (e.g
> > through "ifconfig eth0 hw ether xx:xx:xx:xx:xx:xx"), so every
> > MT8195 SoCs will get their specified mac address in real product.
>
> So this means this is not one MAC address for all SoCs, so this does
> not
> belong to DTSI. Actually it doesn't belong to DTS either. Look how
> others are doing...
OK, will remove it in next send.
>
>
> Best regards,
> Krzysztof
>
