Re: [PATCH net-next v1 01/14] net: phy: nxp-c45-tja11xx: fix the PTP interrupt enablig/disabling

[Radu Pirea (OSS)]

On 16.06.2023 23:36, Andrew Lunn wrote:
> On Fri, Jun 16, 2023 at 04:53:10PM +0300, Radu Pirea (NXP OSS) wrote:
> > .config_intr() handles only the link event interrupt and should
> > disable/enable the PTP interrupt also.
> >
> > It's safe to disable/enable the PTP irq even if the egress ts irq
> > is disabled. This interrupt, the PTP one, acts as a global switch for all
> > PTP irqs.
> >
> > Fixes: 514def5dd339 ("phy: nxp-c45-tja11xx: add timestamping support")
> > CC: stable@xxxxxxxxxxxxxxx # 5.15+
> > Signed-off-by: Radu Pirea (NXP OSS) <radu-nicolae.pirea@xxxxxxxxxxx>
>
> Please don't mix fixes and development work in one patchset. Please
> post this to applying to net, not net-next.

Ok. I will send it to net and apply your suggestions.

> >   static int nxp_c45_config_intr(struct phy_device *phydev)
> >   {
> > -     if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
> > +     /* The return value is ignored on purpose. It might be < 0.
> > +      * 0x807A register is not present on SJA1110 PHYs.
> > +      */
> > +     if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> > +             phy_set_bits_mmd(phydev, MDIO_MMD_VEND1,
> > +                              VEND1_PORT_FUNC_IRQ_EN, PTP_IRQS);
> >                return phy_set_bits_mmd(phydev, MDIO_MMD_VEND1,
> >                                        VEND1_PHY_IRQ_EN, PHY_IRQ_LINK_EVENT);
>
> phy_set_bits_mmd() will not return an error if the register does not
> exist. There is no such indication for MDIO. This is going to do a
> read/modify/write. That read might get 0xffff, or random junk. And
> then the write back will be successful. The only time
> phy_read()/phy_write return error is when there is a problem within
> the bus master, like its clock gets turned off and the transfer times
> out.
>
> So it is good to document you are accessing a register which might not
> exist, but there is no need to ignore the return code.
>
>         Andrew

--
Radu P.