Re: [PATCH v7 phy-next 25/27] phy: include PHY provider header (2/2)

[Vladimir Oltean <vladimir.oltean@nxp.com>]

Sashiko says:
> commit 4194b60e90a65a3b71a94359af9d2b530bae01d8
> Author: Vladimir Oltean <vladimir.oltean@nxp.com>
> phy: include PHY provider header (2/2)
> 
> This commit replaces the inclusion of the generic linux/phy/phy.h header
> with the internal PHY provider header across numerous PHY drivers. It targets
> drivers that act as PHY providers or chained provider/consumers to ensure they
> include the appropriate provider API definitions.
> [ ... ]
> 
> > diff --git a/include/linux/phy/ulpi_phy.h b/include/linux/phy/ulpi_phy.h
> > index 7054b440347ce..0f9e8430d398e 100644
> > --- a/include/linux/phy/ulpi_phy.h
> > +++ b/include/linux/phy/ulpi_phy.h
> > @@ -1,5 +1,5 @@
> >  /* SPDX-License-Identifier: GPL-2.0 */
> > -#include <linux/phy/phy.h>
> > +#include "../../drivers/phy/phy-provider.h"
> 
> Since this header is located in include/linux/phy/, won't navigating up two
> levels resolve to the include/ directory? That would make the compiler search
> for include/drivers/phy/phy-provider.h, which does not exist, and could cause
> build failures.

The LLM review is right that the relative include path is wrong.

> Also, does including a private subsystem header from a public API header in
> include/linux/ cause issues for out-of-tree builds? This exposes private
> structures and could cause regressions for external modules that include
> <linux/phy/ulpi_phy.h> since they won't have access to the kernel's drivers/
> directory.
> 
> If the provider API is needed here, would it be better to move the functions
> that depend on it out of the public header into a .c file?

I have moved the file from include/linux/phy/ulpi_phy.h to drivers/phy/ulpi_phy.h
for v8, and updated the relative include path to just "phy-provider.h".
Frankly, it is an oversight that ulpi_phy.h was still left in the public
<linux/phy/> directory.

> >  /**
> >   * Helper that registers PHY for a ULPI device and adds a lookup for binding it