* Re: [PATCH v11 3/4] phy: Add Sparx5 ethernet serdes PHY driver [not found] ` <20210104082218.1389450-4-steen.hegelund@microchip.com> @ 2021-01-04 12:15 ` Leon Romanovsky 2021-01-04 13:16 ` Steen Hegelund 0 siblings, 1 reply; 3+ messages in thread From: Leon Romanovsky @ 2021-01-04 12:15 UTC (permalink / raw) To: Steen Hegelund Cc: Bjarni Jonasson, Alexandre Belloni, Andrew Lunn, netdev, linux-kernel, Kishon Vijay Abraham I, Vinod Koul, linux-arm-kernel, Microchip UNG Driver List, Lars Povlsen On Mon, Jan 04, 2021 at 09:22:17AM +0100, Steen Hegelund wrote: > Add the Microchip Sparx5 ethernet serdes PHY driver for the 6G, 10G and 25G > interfaces available in the Sparx5 SoC. > > Signed-off-by: Bjarni Jonasson <bjarni.jonasson@microchip.com> > Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com> > Reviewed-by: Andrew Lunn <andrew@lunn.ch> > --- > drivers/phy/Kconfig | 1 + > drivers/phy/Makefile | 1 + > drivers/phy/microchip/Kconfig | 12 + > drivers/phy/microchip/Makefile | 6 + > drivers/phy/microchip/sparx5_serdes.c | 2443 ++++++++++++++++++ > drivers/phy/microchip/sparx5_serdes.h | 129 + > drivers/phy/microchip/sparx5_serdes_regs.h | 2695 ++++++++++++++++++++ > 7 files changed, 5287 insertions(+) > create mode 100644 drivers/phy/microchip/Kconfig > create mode 100644 drivers/phy/microchip/Makefile > create mode 100644 drivers/phy/microchip/sparx5_serdes.c > create mode 100644 drivers/phy/microchip/sparx5_serdes.h > create mode 100644 drivers/phy/microchip/sparx5_serdes_regs.h > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index 00dabe5fab8a..df35c752f3aa 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -70,6 +70,7 @@ source "drivers/phy/ingenic/Kconfig" > source "drivers/phy/lantiq/Kconfig" > source "drivers/phy/marvell/Kconfig" > source "drivers/phy/mediatek/Kconfig" > +source "drivers/phy/microchip/Kconfig" > source "drivers/phy/motorola/Kconfig" > source "drivers/phy/mscc/Kconfig" > source "drivers/phy/qualcomm/Kconfig" > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index 32261e164abd..adac1b1a39d1 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -20,6 +20,7 @@ obj-y += allwinner/ \ > lantiq/ \ > marvell/ \ > mediatek/ \ > + microchip/ \ > motorola/ \ > mscc/ \ > qualcomm/ \ > diff --git a/drivers/phy/microchip/Kconfig b/drivers/phy/microchip/Kconfig > new file mode 100644 > index 000000000000..0b1a818e01b8 <...> > +struct sparx5_sd10g28_args { > + bool skip_cmu_cfg; /* Enable/disable CMU cfg */ > + bool no_pwrcycle; /* Omit initial power-cycle */ > + bool txinvert; /* Enable inversion of output data */ > + bool rxinvert; /* Enable inversion of input data */ > + bool txmargin; /* Set output level to half/full */ > + u16 txswing; /* Set output level */ > + bool mute; /* Mute Output Buffer */ > + bool is_6g; > + bool reg_rst; > +}; All those bools in structs can be squeezed into one u8, just use bitfields, e.g. "u8 a:1;". Also I strongly advise do not do vertical alignment, it will cause to too many churn later when this code will be updated. > + <...> > +static inline void __iomem *sdx5_addr(void __iomem *base[], > + int id, int tinst, int tcnt, > + int gbase, int ginst, > + int gcnt, int gwidth, > + int raddr, int rinst, > + int rcnt, int rwidth) > +{ > +#if defined(CONFIG_DEBUG_KERNEL) > + WARN_ON((tinst) >= tcnt); > + WARN_ON((ginst) >= gcnt); > + WARN_ON((rinst) >= rcnt); > +#endif Please don't put "#if defined(CONFIG_DEBUG_KERNEL)", print WARN_ON(). Thanks _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v11 3/4] phy: Add Sparx5 ethernet serdes PHY driver 2021-01-04 12:15 ` [PATCH v11 3/4] phy: Add Sparx5 ethernet serdes PHY driver Leon Romanovsky @ 2021-01-04 13:16 ` Steen Hegelund 2021-01-04 14:20 ` Leon Romanovsky 0 siblings, 1 reply; 3+ messages in thread From: Steen Hegelund @ 2021-01-04 13:16 UTC (permalink / raw) To: Leon Romanovsky Cc: Bjarni Jonasson, Alexandre Belloni, Andrew Lunn, netdev, linux-kernel, Kishon Vijay Abraham I, Vinod Koul, linux-arm-kernel, Microchip UNG Driver List, Lars Povlsen Hi Leon, On Mon, 2021-01-04 at 14:15 +0200, Leon Romanovsky wrote: > > <...> > > > +struct sparx5_sd10g28_args { > > + bool skip_cmu_cfg; /* Enable/disable CMU cfg > > */ > > + bool no_pwrcycle; /* Omit initial power- > > cycle */ > > + bool txinvert; /* Enable inversion of > > output data */ > > + bool rxinvert; /* Enable inversion of > > input data */ > > + bool txmargin; /* Set output level to > > half/full */ > > + u16 txswing; /* Set output level */ > > + bool mute; /* Mute Output Buffer */ > > + bool is_6g; > > + bool reg_rst; > > +}; > > All those bools in structs can be squeezed into one u8, just use > bitfields, e.g. "u8 a:1;". Got you. > > Also I strongly advise do not do vertical alignment, it will cause to > too many churn later when this code will be updated. So just a single space between the type and the name and the comment is preferable? > > > + > > <...> > > > +static inline void __iomem *sdx5_addr(void __iomem *base[], > > + int id, int tinst, int tcnt, > > + int gbase, int ginst, > > + int gcnt, int gwidth, > > + int raddr, int rinst, > > + int rcnt, int rwidth) > > +{ > > +#if defined(CONFIG_DEBUG_KERNEL) > > + WARN_ON((tinst) >= tcnt); > > + WARN_ON((ginst) >= gcnt); > > + WARN_ON((rinst) >= rcnt); > > +#endif > > Please don't put "#if defined(CONFIG_DEBUG_KERNEL)", print WARN_ON(). OK, I will drop the #if and keep the WARN_ON... > > Thanks Thank you for your comments. BR Steen _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v11 3/4] phy: Add Sparx5 ethernet serdes PHY driver 2021-01-04 13:16 ` Steen Hegelund @ 2021-01-04 14:20 ` Leon Romanovsky 0 siblings, 0 replies; 3+ messages in thread From: Leon Romanovsky @ 2021-01-04 14:20 UTC (permalink / raw) To: Steen Hegelund Cc: Bjarni Jonasson, Alexandre Belloni, Andrew Lunn, netdev, linux-kernel, Kishon Vijay Abraham I, Vinod Koul, linux-arm-kernel, Microchip UNG Driver List, Lars Povlsen On Mon, Jan 04, 2021 at 02:16:38PM +0100, Steen Hegelund wrote: > Hi Leon, > > > On Mon, 2021-01-04 at 14:15 +0200, Leon Romanovsky wrote: > > > > <...> > > > > > +struct sparx5_sd10g28_args { > > > + bool skip_cmu_cfg; /* Enable/disable CMU cfg > > > */ > > > + bool no_pwrcycle; /* Omit initial power- > > > cycle */ > > > + bool txinvert; /* Enable inversion of > > > output data */ > > > + bool rxinvert; /* Enable inversion of > > > input data */ > > > + bool txmargin; /* Set output level to > > > half/full */ > > > + u16 txswing; /* Set output level */ > > > + bool mute; /* Mute Output Buffer */ > > > + bool is_6g; > > > + bool reg_rst; > > > +}; > > > > All those bools in structs can be squeezed into one u8, just use > > bitfields, e.g. "u8 a:1;". > > Got you. > > > > > Also I strongly advise do not do vertical alignment, it will cause to > > too many churn later when this code will be updated. > > So just a single space between the type and the name and the comment is > preferable? Yes, use clang formatter over your code, it will change everything to be aligned to kernel coding style. https://linuxplumbersconf.org/event/7/contributions/803/ > > > > > > + > > > > <...> > > > > > +static inline void __iomem *sdx5_addr(void __iomem *base[], > > > + int id, int tinst, int tcnt, > > > + int gbase, int ginst, > > > + int gcnt, int gwidth, > > > + int raddr, int rinst, > > > + int rcnt, int rwidth) > > > +{ > > > +#if defined(CONFIG_DEBUG_KERNEL) > > > + WARN_ON((tinst) >= tcnt); > > > + WARN_ON((ginst) >= gcnt); > > > + WARN_ON((rinst) >= rcnt); > > > +#endif > > > > Please don't put "#if defined(CONFIG_DEBUG_KERNEL)", print WARN_ON(). > > OK, I will drop the #if and keep the WARN_ON... Thanks _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-04 14:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20210104082218.1389450-1-steen.hegelund@microchip.com>
[not found] ` <20210104082218.1389450-4-steen.hegelund@microchip.com>
2021-01-04 12:15 ` [PATCH v11 3/4] phy: Add Sparx5 ethernet serdes PHY driver Leon Romanovsky
2021-01-04 13:16 ` Steen Hegelund
2021-01-04 14:20 ` Leon Romanovsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).