On Tue, 2024-01-23 at 18:37 +0000, Conor Dooley wrote: > On Tue, Jan 23, 2024 at 06:30:16PM +0100, Philippe Schenker wrote: > > > > > > On Tue, 2024-01-23 at 17:23 +0000, Conor Dooley wrote: > > > On Tue, Jan 23, 2024 at 05:17:53PM +0100, Philippe Schenker wrote: > > > > > > > > > > > > On Tue, 2024-01-23 at 16:06 +0000, Conor Dooley wrote: > > > > > On Tue, Jan 23, 2024 at 02:50:13PM +0100, Philippe Schenker > > > > > wrote: > > > > > > From: Philippe Schenker <philippe.schenker@xxxxxxxxxxxx> > > > > > > > > > > > > This commit adds the dt-binding for KSZ8567, a robust 7-port > > > > > > Ethernet switch. The KSZ8567 features two RGMII/MII/RMII > > > > > > interfaces, > > > > > > each capable of gigabit speeds, complemented by five 10/100 > > > > > > Mbps > > > > > > MAC/PHYs. > > > > > > > > > > > > Signed-off-by: Philippe Schenker > > > > > > <philippe.schenker@xxxxxxxxxxxx> > > > > > > > > > > This device has all the same constraints as the other ones in > > > > > this > > > > > binding, why is it not compatible with any of them? If it > > > > > isn't, > > > > > the > > > > > compatible should mention why it is not. > > > > > > > > Hi Conor, Thanks for your message! > > > > > > > > I need the compatible to make sure the correct ID of the switch > > > > is > > > > being set in the driver as well as its features. > > > > > > Are the features of this switch such that a driver for another ksz > > > switch would not work (even in a limited capacity) with the 8567? > > > Things like the register map changing or some feature being > > > removed > > > are > > > examples of why it may not work. > > > > Yes the ksz dsa driver is made so that it checks the ID of the > > attached > > chip and refuses to work if it doesn't match. [1] > > That sounds counter productive to be honest. Why does the driver not > trust that the dt is correct? I saw this recently in some IIO drivers, > but it was shot down for this sort of reason. > > > It is a very similar chip and uses the same regmap as KSZ9567 but > > with > > lower phy-speeds on its 5 switch ports. The two upstream CPU ports > > are > > gigabit capable. All this information is set-up in the second patch > > of > > this series. [2] > > That, to me, means the lack of a fallback is justified. If it were the > other way around, then a fallback sounds like it would be suitable. Yes, I believe a generic fallback would anyway mostly end up with errors or misbehavior. But I also might be wrong. What I checked is that these per-switch settings even increased from 5.15..6.7 which to me it seems those are really needed. I am no expert in this driver at all I just try to make my hardware work and with these patches it now seems to switch really well. > > > > > I will include a description to the second series. Thanks for your > > feedback. > > Okay, thanks. You can add > Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> > when you do. > > And despite the email, I have nothing to do with these switches, I am > just a sucker that signed up to review dt-bindings... Well, I'm grateful for your review and feedback. Thanks for your work around Linux kernel! And thanks for the Acked-by. Philippe > > Thanks, > Conor. > > > > > Philippe > > > > > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/dsa/microchip/ksz_common.c?h=v6.8-rc1#n3181 > > [2] > > https://patchwork.kernel.org/project/netdevbpf/patch/20240123135014.614858-2-dev@xxxxxxxxxxxx/ > > > > > > > > > You mean I shall mention the reason in the commit-message, or > > > > where? > > > > > > Yes. > > > > > > Thanks, > > > Conor > > > > > > > > > Documentation/devicetree/bindings/net/dsa/microchip,ksz.yam > > > > > > l | > > > > > > 1 + > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.ya > > > > > > ml > > > > > > b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.ya > > > > > > ml > > > > > > index c963dc09e8e1..52acc15ebcbf 100644 > > > > > > --- > > > > > > a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.ya > > > > > > ml > > > > > > +++ > > > > > > b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.ya > > > > > > ml > > > > > > @@ -31,6 +31,7 @@ properties: > > > > > > - microchip,ksz9893 > > > > > > - microchip,ksz9563 > > > > > > - microchip,ksz8563 > > > > > > + - microchip,ksz8567 > > > > > > > > > > > > reset-gpios: > > > > > > description: > > > > > > -- > > > > > > 2.34.1 > > > > > >
