netdev - Re: [PATCH net-next] net: ethernet: mediatek: Allow gaps in MAC allocation

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: <2e441889-cb53-4004-aeed-1268d20b54b7@intel.com>
Date: Fri, 5 Jul 2024 14:55:35 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Daniel Golle <daniel@...rotopia.org>, Elad Yifee <eladwf@...il.com>
CC: Felix Fietkau <nbd@....name>, Sean Wang <sean.wang@...iatek.com>, Mark Lee
	<Mark-MC.Lee@...iatek.com>, Lorenzo Bianconi <lorenzo@...nel.org>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Matthias
 Brugger <matthias.bgg@...il.com>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH net-next] net: ethernet: mediatek: Allow gaps in MAC
 allocation

On 7/5/24 13:24, Daniel Golle wrote:
> Hi netdev maintainers,
> 

TL;DR
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>

and no-one provided feedback against merging this patch so far

> On Tue, Jul 02, 2024 at 09:05:19AM +0000, Daniel Golle wrote:
>>> what about:
>>> 4733│ static int mtk_sgmii_init(struct mtk_eth *eth)
>>> 4734│ {
>>> 4735│         struct device_node *np;
>>> 4736│         struct regmap *regmap;
>>> 4737│         u32 flags;
>>> 4738│         int i;
>>> 4739│
>>> 4740│         for (i = 0; i < MTK_MAX_DEVS; i++) {
>>> 4741│                 np = of_parse_phandle(eth->dev->of_node, "mediatek,sgmiisys", i);
>>> 4742│                 if (!np)
>>> 4743│                         break;
>>>
>>> should we also continue here?
>>
>> Good point. As sgmiisys is defined in dtsi it's not so relevant in
>> practise though, as the SoC components are of course always present even
>> if we don't use them. Probably it is still better to not be overly
>> strict on the presence of things we may not even use, not even emit an
>> error message and silently break something else, so yes, worth fixing
>> imho.
>>
> 
> I've noticed that this patch was marked as "Changes Requested" on patchwork
> despite having received a positive review.
> 
> I'm afraid this is possibly due to a misunderstanding:
> 
> The (unrelated and rather exotic) similar issue pointed to by Przemek
> Kitszel should not be fixed in the same commit. It is unrelated, and if
> at all, should be sent to 'net' tree rather than 'net-next'.
> 
> Looking at it more closely I would not consider it an issue as we
> currently in the bindings we **require** the correct number of sgmiisys phandles to be
> present for each SoC supporting SGMII:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/mediatek,net.yaml#n200
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/mediatek,net.yaml#n245
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/mediatek,net.yaml#n287
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/mediatek,net.yaml#n325
> 
> Hence there aren't ever any gaps, also because the sgmiisys phandles are
> defined in the SoC-specific DTSI **even for boards not making any use of
> them**.
> 
> I hence would like this very patch to be merged (or at least discussed)
> as-is, and if there is really a need to address the issue mentioned by
> Przemek Kitszel, then deal with it in a separate commit.
> 
> 
> Cheers
> 
> 
> Daniel

you are right, I have even marked this specifically as Reviewed-by: $me,
so I think is just a mistake to mark it as CR
(without the RB tag, it will be wise to mark such comments as no-issue
or similar, and I typically do so)