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

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

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

TL;DR
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.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)