Re: [PATCH v3 0/4] phy: phy-can-transceiver: Ad-hoc cleanups and refactoring

Re: [PATCH v3 0/4] phy: phy-can-transceiver: Ad-hoc cleanups and refactoring

[Vinod Koul]

On 04-05-26, 08:58, Andy Shevchenko wrote:
> The driver does two things that need to be addressed:
> - includes subject to remove gpio.h
> - checks for error code from device property APIs when it can be done in
>   a robust way
> 
> This series addresses the above and adds a couple of additional refactoring.

Sashiko flagged some issues, some of them not introduced by this, can
you please check this:

https://sashiko.dev/#/patchset/20260504070054.29508-1-andriy.shevchenko%40linux.intel.com

-- 
~Vinod

Re: [PATCH v3 0/4] phy: phy-can-transceiver: Ad-hoc cleanups and refactoring

[Andy Shevchenko]

On Sun, May 10, 2026 at 04:21:38PM +0530, Vinod Koul wrote:
> On 04-05-26, 08:58, Andy Shevchenko wrote:
> > The driver does two things that need to be addressed:
> > - includes subject to remove gpio.h
> > - checks for error code from device property APIs when it can be done in
> >   a robust way
> > 
> > This series addresses the above and adds a couple of additional refactoring.
> 
> Sashiko flagged some issues, some of them not introduced by this, can
> you please check this:
> 
> https://sashiko.dev/#/patchset/20260504070054.29508-1-andriy.shevchenko%40linux.intel.com

"Could this result in a null pointer dereference if device_get_match_data()
returns null?"
Yes, it sounds legit but not introduced here.

"In the original code, the warning was suppressed when the property was missing
because err evaluated to -EINVAL. Now, if the property is absent, max_bitrate
is explicitly set to 0 in the else block, which then unconditionally triggers
this warning."
True, but I don't know which is better here, I consider that it's
good to inform user about default being used as a fallback. I can change
this back to the original logic. What do you prefer?

The third one is the repetition of the first one (see above).

TL;DR: The only one legitimated question is about a (new old) warning.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/4] phy: phy-can-transceiver: Ad-hoc cleanups and refactoring

[Vinod Koul]

On 10-05-26, 15:51, Andy Shevchenko wrote:
> On Sun, May 10, 2026 at 04:21:38PM +0530, Vinod Koul wrote:
> > On 04-05-26, 08:58, Andy Shevchenko wrote:
> > > The driver does two things that need to be addressed:
> > > - includes subject to remove gpio.h
> > > - checks for error code from device property APIs when it can be done in
> > >   a robust way
> > > 
> > > This series addresses the above and adds a couple of additional refactoring.
> > 
> > Sashiko flagged some issues, some of them not introduced by this, can
> > you please check this:
> > 
> > https://sashiko.dev/#/patchset/20260504070054.29508-1-andriy.shevchenko%40linux.intel.com
> 
> "Could this result in a null pointer dereference if device_get_match_data()
> returns null?"
> Yes, it sounds legit but not introduced here.
> 
> "In the original code, the warning was suppressed when the property was missing
> because err evaluated to -EINVAL. Now, if the property is absent, max_bitrate
> is explicitly set to 0 in the else block, which then unconditionally triggers
> this warning."
> True, but I don't know which is better here, I consider that it's
> good to inform user about default being used as a fallback. I can change
> this back to the original logic. What do you prefer?
> 
> The third one is the repetition of the first one (see above).
> 
> TL;DR: The only one legitimated question is about a (new old) warning.

Yeah would be great if we could fix these as well please

-- 
~Vinod

Re: [PATCH v3 0/4] phy: phy-can-transceiver: Ad-hoc cleanups and refactoring

[Andy Shevchenko]

On Mon, May 11, 2026 at 09:47:42PM +0530, Vinod Koul wrote:
> On 10-05-26, 15:51, Andy Shevchenko wrote:
> > On Sun, May 10, 2026 at 04:21:38PM +0530, Vinod Koul wrote:
> > > On 04-05-26, 08:58, Andy Shevchenko wrote:
> > > > The driver does two things that need to be addressed:
> > > > - includes subject to remove gpio.h
> > > > - checks for error code from device property APIs when it can be done in
> > > >   a robust way
> > > > 
> > > > This series addresses the above and adds a couple of additional refactoring.
> > > 
> > > Sashiko flagged some issues, some of them not introduced by this, can
> > > you please check this:
> > > 
> > > https://sashiko.dev/#/patchset/20260504070054.29508-1-andriy.shevchenko%40linux.intel.com
> > 
> > "Could this result in a null pointer dereference if device_get_match_data()
> > returns null?"
> > Yes, it sounds legit but not introduced here.
> > 
> > "In the original code, the warning was suppressed when the property was missing
> > because err evaluated to -EINVAL. Now, if the property is absent, max_bitrate
> > is explicitly set to 0 in the else block, which then unconditionally triggers
> > this warning."
> > True, but I don't know which is better here, I consider that it's
> > good to inform user about default being used as a fallback. I can change
> > this back to the original logic. What do you prefer?
> > 
> > The third one is the repetition of the first one (see above).
> > 
> > TL;DR: The only one legitimated question is about a (new old) warning.
> 
> Yeah would be great if we could fix these as well please

I just sent a v4:
20260512130552.272476-1-andriy.shevchenko@linux.intel.com

-- 
With Best Regards,
Andy Shevchenko