Re: [PATCH v5 net-next 08/13] net: phy: marvell-88q2xxx: add support for temperature sensor

[Guenter Roeck]

On 1/31/24 23:11, Dimitri Fedrau wrote:
> Am Wed, Jan 31, 2024 at 04:17:06PM +0100 schrieb Andrew Lunn:
> > > +static int mv88q2xxx_hwmon_probe(struct phy_device *phydev)
> > > +{
> > > +	struct device *dev = &phydev->mdio.dev;
> > > +	struct device *hwmon;
> > > +	char *hwmon_name;
> > > +	int ret;
> > > +
> > > +	/* Enable temperature sensor interrupt */
> > > +	ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS,
> > > +			       MDIO_MMD_PCS_MV_TEMP_SENSOR1,
> > > +			       MDIO_MMD_PCS_MV_TEMP_SENSOR1_INT_EN);
> >
> > You enable an interrupt, but i don't see any changes to the interrupt
> > handler to handle any interrupts which are generated?
> Hi Andrew,
>
> you are right. Have to remove these lines. Besides enabling the interrupt
> in MDIO_MMD_PCS_MV_TEMP_SENSOR1, there are two further register writes
> necessary to make the interrupt propagate. I didn't want it to propagate.
> Anyway it's wrong. I couldn't find a good solution to use the temperature
> interrupt. Will have a look into this, and probably figuring out how to
> do so. But it won't be part of this patch series.

From hwmon perspective, the expected use of such an interrupt would be
to call hwmon_notify_event() with the affected limit attribute as argument.
This would notify the thermal subsystem if the sensor is registered with it
(your patch doesn't set the necessary flag when registering the driver,
so this would not happen), it will send a notification to the sysfs
attribute, and generate a udev event.

Guenter