> I am planning to have following commit msg;
>
>
>
> “This feature is not used by our mainstream customers as they have additional
As i said, this is not your driver, for you customers. It is the Linux
kernel driver. Please drop all references to your customers. If you
need to address anybody, it should be the Linux community as a whole,
or maybe the users of this driver.
> mechanism to monitor the supply at System level as accuracy requirements are
> different for each application. The device is designed with inbuilt monitor
> with interrupt disabled by default and let user choose if they want to exercise
> the monitor. However, the driver had this interrupt enabled, the request here
> is disable it by default in driver however not change in datasheet. Let user
> of the driver review the accuracy offered by monitor and if meets the
> expectation, they can always enable it.”
I would much more prefer something like...
The over voltage interrupt is enabled, but if it every occurs, there is
no code to make use of it. So remove the pointless enabling of it. It
can re-enabled when HWMON support is added. For the same reason,
enabling of the interrupts DP83811_RX_ERR_HF_INT_EN,
DP83811_MS_TRAINING_INT_EN, DP83811_ESD_EVENT_INT_EN,
DP83811_ENERGY_DET_INT_EN, DP83811_LINK_QUAL_INT_EN,
DP83811_JABBER_DET_INT_EN, DP83811_POLARITY_INT_EN,
DP83811_SLEEP_MODE_INT_EN, DP83811_OVERTEMP_INT_EN,
DP83811_UNDERVOLTAGE_INT_EN is also removed, since there is no code
which acts on these interrupts.
And update the patch to fit.
Andrew
Hi Andrew,
As mentioned we want to do this in phases:
a) this patch to disable the Overvoltage driver interrupt
b) After carefully considering other interrupts, plan a follow-on patch to take care of other interrupts.
Patch comment will be inline with your suggestion
"The over voltage interrupt is enabled, but if it ever occurs, there is no code to make use of it. So removing it. It
can re-enabled when HWMON support is added "
Regarfds,
Geet
On 9/9/21, 1:37 PM, "Andrew Lunn" <[email protected]> wrote:
> I am planning to have following commit msg;
>
>
>
> “This feature is not used by our mainstream customers as they have additional
As i said, this is not your driver, for you customers. It is the Linux
kernel driver. Please drop all references to your customers. If you
need to address anybody, it should be the Linux community as a whole,
or maybe the users of this driver.
> mechanism to monitor the supply at System level as accuracy requirements are
> different for each application. The device is designed with inbuilt monitor
> with interrupt disabled by default and let user choose if they want to exercise
> the monitor. However, the driver had this interrupt enabled, the request here
> is disable it by default in driver however not change in datasheet. Let user
> of the driver review the accuracy offered by monitor and if meets the
> expectation, they can always enable it.”
I would much more prefer something like...
The over voltage interrupt is enabled, but if it every occurs, there is
no code to make use of it. So remove the pointless enabling of it. It
can re-enabled when HWMON support is added. For the same reason,
enabling of the interrupts DP83811_RX_ERR_HF_INT_EN,
DP83811_MS_TRAINING_INT_EN, DP83811_ESD_EVENT_INT_EN,
DP83811_ENERGY_DET_INT_EN, DP83811_LINK_QUAL_INT_EN,
DP83811_JABBER_DET_INT_EN, DP83811_POLARITY_INT_EN,
DP83811_SLEEP_MODE_INT_EN, DP83811_OVERTEMP_INT_EN,
DP83811_UNDERVOLTAGE_INT_EN is also removed, since there is no code
which acts on these interrupts.
And update the patch to fit.
Andrew
On Fri, Sep 10, 2021 at 12:41:53AM +0000, Modi, Geet wrote:
> Hi Andrew,
>
> As mentioned we want to do this in phases:
> a) this patch to disable the Overvoltage driver interrupt
> b) After carefully considering other interrupts, plan a follow-on patch to take care of other interrupts.
I still don't get it. Why just Over volt now and not the rest, which
are equally useless? It makes me think there is something seriously
wrong with over voltage, which you are not telling us about. Maybe an
interrupt storm? If there is something broken here, this patch needs
to be back ported to stable.
Andrew
Hi Andrew,
Please be assure the monitors are part of the PHY and well captured in device datasheet. The only reason to go selectively is as we have not carefully reviewed the other interrupts usage by application, hence don't want to make the change in haste.
Regards,
Geet
On 9/9/21, 6:31 PM, "Andrew Lunn" <[email protected]> wrote:
On Fri, Sep 10, 2021 at 12:41:53AM +0000, Modi, Geet wrote:
> Hi Andrew,
>
> As mentioned we want to do this in phases:
> a) this patch to disable the Overvoltage driver interrupt
> b) After carefully considering other interrupts, plan a follow-on patch to take care of other interrupts.
I still don't get it. Why just Over volt now and not the rest, which
are equally useless? It makes me think there is something seriously
wrong with over voltage, which you are not telling us about. Maybe an
interrupt storm? If there is something broken here, this patch needs
to be back ported to stable.
Andrew