Re: Update driver xdpe152c4.c

Guenter Roeck
2025-04-08T04:06:07Z

On 4/7/25 20:28, Ashish.Yadav@infineon.com wrote:
> Hi Guenter, Wolfram and Mukesh,
>
> I hope you are doing well.
> Thanks for your suggestion/feedback.
>
> Sorry for the inconvenience.
> This patch is still in the testing phase and not the final one, that is why " pr_warn()" is used for debug purposes.
>
> We will get back to you with the final patch with proper process as suggested by you.
>

Please provide an answer for:

> I'd like to know what causes the problem before applying a change like this.
> I suspect that the chip needs either a delay between accesses or a
> delay after a write. Both are supported by the PMBus core.

There are various microcontroller based PMBus chips. Those microcontrollers
sometimes need a delay between accesses to work properly. That is why
access_delay, write_delay, and now page_change_delay are supported by the
PMBus core. It is at least somewhat likely that the xdpe152c4 has this
problem. The delay between retries in your code is an indication that some
delay between accesses may indeed be needed.

Another concern is the need for retry due to PEC errors. That would normally
be due to a bad board design, which is not chip dependent.

Long story short, so far not a single PMBus chip needs retries. It is very unlikely
that this chip is different. You'll have to provide substantially more details
for me to accept retries instead of access/write delays in the driver.

Thanks,
Guenter

> With Best Regards
> Ashish Yadav
>
>
> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: 07 April 2025 20:06
> To: Wolfram Sang <wsa+renesas@sang-engineering.com>; Lin Shirley (SMD C3 GC TM DCO) <Shirley.Lin@infineon.com>; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; jdelvare@suse.com; corbet@lwn.net; patrick.rudolph@9elements.com; bhelgaas@google.com; ninad@linux.ibm.com; festevam@denx.de; devicetree@vger.kernel.org; linux-hwmon@vger.kernel.org; linux-i2c@vger.kernel.org; Liu Mills (SMD C3 GC TM DCO) <Mills.Liu@infineon.com>; Yadav Ashish (PSS PCS RD FW HD) <Ashish.Yadav@infineon.com>; Fang Ian (SMD C3 GC TM DCO) <Ian.Fang@infineon.com>
> Subject: Re: Update driver xdpe152c4.c
>
> Caution: This e-mail originated outside Infineon Technologies. Please be cautious when sharing information or opening attachments especially from unknown senders. Refer to our intranet guide<https://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx> to help you identify Phishing email.
>
>
>
> On 4/7/25 07:01, Guenter Roeck wrote:
>> On 4/6/25 23:15, Wolfram Sang wrote:
>>>
>>>> Please help to review the attached Linux Kernel patch for xdpe152xx driver.
>>>
>>> To shorten things, you could return -EAGAIN as an error code, then
>>> the I2C core will retry the message for you. Within the configured
>>> limits for the controller.
>>>
>>
>> The patch neglects to state the _reason_ for the change.
>>
>> I'd like to know what causes the problem before applying a change like this.
>> I suspect that the chip needs either a delay between accesses or a
>> delay after a write. Both are supported by the PMBus core.
>>
>
> And this is a complete no-go:
>
> + if (ret < 0)
> + pr_warn("PMBUS READ ERROR:%d\n", ret);
>
> It isn't even dev_warn(), and I am not going to accept patches which will clog the kernel log for every single error return. Just imagine how the kernel log would look like if every driver would log errors like this.
>
> Guenter
>
>