| Message ID | 20231026094856.986796-1-0x1207@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v1,1/1] net: stmmac: xgmac: Enable support for multiple Flexible PPS outputs | expand |
On 10/26/2023 2:48 AM, Furong Xu wrote: > From XGMAC Core 3.20 and later, each Flexible PPS has individual PPSEN bit > to select Fixed mode or Flexible mode. The PPSEN must be set, or it stays > in Fixed PPS mode by default. > XGMAC Core prior 3.20, corresponding PPSEN bits are read-only reserved, > always set PPSEN do not make things worse ;) > Previous revisions the corresponding bits are always set, and don't get modified by writes, so setting these bits for all hardware has no ill effect. In the previous code we always set BIT(4), but nwo we set BIT(4+x). This won't affect XGMAC prior to 3.20, but corrects a mistake when programming the newer XGMAC. Ok. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> The original code was added in 95eaf3cd0a90 ("net: stmmac: dwxgmac: Add Flexible PPS support"), which landed in v5.4 It looks like XGMAC Core 3.20 support was not added until possibly commit 669a55560e4b ("net: stmmac: Check more MAC HW features for XGMAC Core 3.20") which appears to be new enough that its not in any official Linux release, though it looks like it was already in net. Perhaps this should be tagged Fixes: and sent through net, hopefully to try and hit 6.6 or at least a stable release shortly after? Thanks, Jake
On Thu, Oct 26, 2023 at 05:48:56PM +0800, Furong Xu wrote: > From XGMAC Core 3.20 and later, each Flexible PPS has individual PPSEN bit > to select Fixed mode or Flexible mode. The PPSEN must be set, or it stays > in Fixed PPS mode by default. Are you sure 3.10a don't have the PPSEN flag available for all outputs too? > XGMAC Core prior 3.20, corresponding PPSEN bits are read-only reserved, > always set PPSEN do not make things worse ;) > > Signed-off-by: Furong Xu <0x1207@gmail.com> > --- > drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 +- > drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h > index 7a8f47e7b728..a4e8b498dea9 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h > @@ -259,7 +259,7 @@ > ((val) << XGMAC_PPS_MINIDX(x)) > #define XGMAC_PPSCMD_START 0x2 > #define XGMAC_PPSCMD_STOP 0x5 > -#define XGMAC_PPSEN0 BIT(4) > +#define XGMAC_PPSENx(x) BIT(4 + (x) * 8) > #define XGMAC_PPSx_TARGET_TIME_SEC(x) (0x00000d80 + (x) * 0x10) > #define XGMAC_PPSx_TARGET_TIME_NSEC(x) (0x00000d84 + (x) * 0x10) > #define XGMAC_TRGTBUSY0 BIT(31) > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > index f352be269deb..53bb8f16c481 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > @@ -1178,7 +1178,7 @@ static int dwxgmac2_flex_pps_config(void __iomem *ioaddr, int index, > > val |= XGMAC_PPSCMDx(index, XGMAC_PPSCMD_START); > val |= XGMAC_TRGTMODSELx(index, XGMAC_PPSCMD_START); > - val |= XGMAC_PPSEN0; > + val |= XGMAC_PPSENx(index); At the very least it would be nice to have a comment here that the mode selection was available for the output #0 only in the IP-cores prior v3.20a with the outputs 1-3 always working as flexible PPS outputs. Other than that no more comments: Reviewed-by: Serge Semin <fancer.lancer@gmail.com> -Serge(y) > > writel(cfg->start.tv_sec, ioaddr + XGMAC_PPSx_TARGET_TIME_SEC(index)); > > -- > 2.34.1 > >
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index 7a8f47e7b728..a4e8b498dea9 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -259,7 +259,7 @@ ((val) << XGMAC_PPS_MINIDX(x)) #define XGMAC_PPSCMD_START 0x2 #define XGMAC_PPSCMD_STOP 0x5 -#define XGMAC_PPSEN0 BIT(4) +#define XGMAC_PPSENx(x) BIT(4 + (x) * 8) #define XGMAC_PPSx_TARGET_TIME_SEC(x) (0x00000d80 + (x) * 0x10) #define XGMAC_PPSx_TARGET_TIME_NSEC(x) (0x00000d84 + (x) * 0x10) #define XGMAC_TRGTBUSY0 BIT(31) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index f352be269deb..53bb8f16c481 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -1178,7 +1178,7 @@ static int dwxgmac2_flex_pps_config(void __iomem *ioaddr, int index, val |= XGMAC_PPSCMDx(index, XGMAC_PPSCMD_START); val |= XGMAC_TRGTMODSELx(index, XGMAC_PPSCMD_START); - val |= XGMAC_PPSEN0; + val |= XGMAC_PPSENx(index); writel(cfg->start.tv_sec, ioaddr + XGMAC_PPSx_TARGET_TIME_SEC(index));
From XGMAC Core 3.20 and later, each Flexible PPS has individual PPSEN bit to select Fixed mode or Flexible mode. The PPSEN must be set, or it stays in Fixed PPS mode by default. XGMAC Core prior 3.20, corresponding PPSEN bits are read-only reserved, always set PPSEN do not make things worse ;) Signed-off-by: Furong Xu <0x1207@gmail.com> --- drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 +- drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)