On Fri, May 25, 2018 at 04:32:52PM +0100, Jose Abreu wrote:
> +int dwmac5_pps_config(void __iomem *ioaddr, bool enable)
> +{
> + u32 val = readl(ioaddr + MAC_PPS_CONTROL);
> +
> + /* There is no way to disable fixed PPS output so we just reset
> + * the values to make sure its in fixed PPS mode */
In that case, don't try to make this appear to be programmable. Just
reset the registers in your probe or setup function unconditionally.
> + val &= ~PPSx_MASK(0);
> + val |= TRGTMODSELx(0, 0x2);
> +
> + writel(val, ioaddr + MAC_PPS_CONTROL);
> + return 0;
> +}
> +
> +int dwmac5_flex_pps_config(void __iomem *ioaddr, int index,
> + struct stmmac_pps_cfg *cfg, bool enable,
> + u32 sub_second_inc, u32 systime_flags)
> +{
> + u32 tnsec = readl(ioaddr + MAC_PPSx_TARGET_TIME_NSEC(index));
> + u32 val = readl(ioaddr + MAC_PPS_CONTROL);
> + u64 period;
> +
> + if (!cfg->available)
> + return -EINVAL;
> + if (tnsec & TRGTBUSY0)
> + return -EBUSY;
> + if (!sub_second_inc || !systime_flags)
> + return -EINVAL;
Don't add tests on the arguments like this. Instead, make sure the
caller always provides correct values.
> +
> + val &= ~PPSx_MASK(index);
> +
> + if (!enable) {
> + val |= PPSCMDx(index, 0x5);
> + writel(val, ioaddr + MAC_PPS_CONTROL);
> + return 0;
> + }
> +
> + val |= PPSCMDx(index, 0x2);
> + val |= TRGTMODSELx(index, 0x2);
> + val |= PPSEN0;
> +
> + writel(cfg->start.tv_sec, ioaddr + MAC_PPSx_TARGET_TIME_SEC(index));
> +
> + if (!(systime_flags & PTP_TCR_TSCTRLSSR))
> + cfg->start.tv_nsec = (cfg->start.tv_nsec * 1000) / 465;
> + writel(cfg->start.tv_nsec, ioaddr + MAC_PPSx_TARGET_TIME_NSEC(index));
> +
> + period = cfg->period.tv_sec * 1000000000;
> + period += cfg->period.tv_nsec;
> + struct timespec64 period;
> +
> + do_div(period, sub_second_inc);
> +
> + if (period <= 1)
> + return -EINVAL;
> +
> + writel(period - 1, ioaddr + MAC_PPSx_INTERVAL(index));
> +
> + period >>= 1;
> + if (period <= 1)
> + return -EINVAL;
> +
> + writel(period - 1, ioaddr + MAC_PPSx_WIDTH(index));
> +
> + /* Finally, activate it */
> + writel(val, ioaddr + MAC_PPS_CONTROL);
> + return 0;
> +}
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> index 7d3a5c7..35c6d0c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
> @@ -140,19 +140,50 @@ static int stmmac_set_time(struct ptp_clock_info *ptp,
> static int stmmac_enable(struct ptp_clock_info *ptp,
> struct ptp_clock_request *rq, int on)
> {
> - return -EOPNOTSUPP;
> + struct stmmac_priv *priv =
> + container_of(ptp, struct stmmac_priv, ptp_clock_ops);
> + struct stmmac_pps_cfg *cfg;
> + int ret = -EOPNOTSUPP;
> + unsigned long flags;
> +
> + switch (rq->type) {
> + case PTP_CLK_REQ_PEROUT:
> + cfg = &priv->pps[rq->perout.index];
> +
> + cfg->start.tv_sec = rq->perout.start.sec;
> + cfg->start.tv_nsec = rq->perout.start.nsec;
> + cfg->period.tv_sec = rq->perout.period.sec;
> + cfg->period.tv_nsec = rq->perout.period.nsec;
> +
> + spin_lock_irqsave(&priv->ptp_lock, flags);
> + ret = stmmac_flex_pps_config(priv, priv->ioaddr,
> + rq->perout.index, cfg, on,
> + priv->sub_second_inc,
> + priv->systime_flags);
> + spin_unlock_irqrestore(&priv->ptp_lock, flags);
> + break;
> + case PTP_CLK_REQ_PPS:
> + spin_lock_irqsave(&priv->ptp_lock, flags);
> + ret = stmmac_pps_config(priv, priv->ioaddr, on);
This is not what PTP_CLK_REQ_PPS is for. It only to arrange for a
trigger into the kernel's PPS sub-system.
[ Sorry that the ptp header files don't explain this in the comments.
I should really fix that. ]
> + spin_unlock_irqrestore(&priv->ptp_lock, flags);
> + break;
> + default:
> + break;
> + }
> +
> + return ret;
> }
>
> /* structure describing a PTP hardware clock */
> -static const struct ptp_clock_info stmmac_ptp_clock_ops = {
> +static struct ptp_clock_info stmmac_ptp_clock_ops = {
> .owner = THIS_MODULE,
> .name = "stmmac_ptp_clock",
> .max_adj = 62500000,
> .n_alarm = 0,
> .n_ext_ts = 0,
> - .n_per_out = 0,
> + .n_per_out = 0, /* will be overwritten in stmmac_ptp_register */
> .n_pins = 0,
> - .pps = 0,
> + .pps = 0, /* will be overwritten in stmmac_ptp_register */
Again, this is for inputting a PPS event into the kernel's PPS subsystem.
Thanks,
Richard
