[PATCH] usb: dwc3-of-simple: Stop using of_reset_control_array_get() directly

[PATCH] usb: dwc3-of-simple: Stop using of_reset_control_array_get() directly

[Philipp Zabel]

Use of_reset_control_array_get_optional_exclusive() instead, it is
implemented as:

  static inline struct reset_control *
  of_reset_control_array_get_optional_exclusive(struct device_node *node)
  {
          return of_reset_control_array_get(node, false, true, true);
  }

This makes the code easier to understand and removes the last remaining
direct use of of_reset_control_array_get(). No functional changes.

Fixes: f4cc91ddd856 ("usb: dwc3: of-simple: remove Amlogic GXL and AXG compatibles")
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/usb/dwc3/dwc3-of-simple.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index d1539fc9eabd..9cf9ee1b637b 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -52,8 +52,7 @@ static int dwc3_of_simple_probe(struct platform_device *pdev)
 	if (of_device_is_compatible(np, "rockchip,rk3399-dwc3"))
 		simple->need_reset = true;
 
-	simple->resets = of_reset_control_array_get(np, false, true,
-						    true);
+	simple->resets = of_reset_control_array_get_optional_exclusive(np);
 	if (IS_ERR(simple->resets)) {
 		ret = PTR_ERR(simple->resets);
 		dev_err(dev, "failed to get device resets, err=%d\n", ret);

---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240124-dwc3-of-simple-reset-control-array-fix-e4e9822028dd

Best regards,
-- 
Philipp Zabel <p.zabel@pengutronix.de>

Re: [PATCH] usb: dwc3-of-simple: Stop using of_reset_control_array_get() directly

[Greg Kroah-Hartman]

On Wed, Jan 24, 2024 at 12:26:20PM +0100, Philipp Zabel wrote:
> Use of_reset_control_array_get_optional_exclusive() instead, it is
> implemented as:
> 
>   static inline struct reset_control *
>   of_reset_control_array_get_optional_exclusive(struct device_node *node)
>   {
>           return of_reset_control_array_get(node, false, true, true);
>   }
> 
> This makes the code easier to understand and removes the last remaining
> direct use of of_reset_control_array_get(). No functional changes.
> 
> Fixes: f4cc91ddd856 ("usb: dwc3: of-simple: remove Amlogic GXL and AXG compatibles")

No functional change, but a Fixes: tag?  That doesn't make sense to me,
sorry.

thanks,

greg k-h

Re: [PATCH] usb: dwc3-of-simple: Stop using of_reset_control_array_get() directly

[Greg Kroah-Hartman]

On Wed, Jan 24, 2024 at 12:26:20PM +0100, Philipp Zabel wrote:
> Use of_reset_control_array_get_optional_exclusive() instead, it is
> implemented as:
> 
>   static inline struct reset_control *
>   of_reset_control_array_get_optional_exclusive(struct device_node *node)
>   {
>           return of_reset_control_array_get(node, false, true, true);
>   }
> 
> This makes the code easier to understand and removes the last remaining
> direct use of of_reset_control_array_get(). No functional changes.

Does this mean the function should be removed or made static now?

thanks,

greg k-h

Re: [PATCH] usb: dwc3-of-simple: Stop using of_reset_control_array_get() directly

[Philipp Zabel]

On Mi, 2024-01-24 at 04:39 -0800, Greg Kroah-Hartman wrote:
> On Wed, Jan 24, 2024 at 12:26:20PM +0100, Philipp Zabel wrote:
> > Use of_reset_control_array_get_optional_exclusive() instead, it is
> > implemented as:
> > 
> >   static inline struct reset_control *
> >   of_reset_control_array_get_optional_exclusive(struct device_node *node)
> >   {
> >           return of_reset_control_array_get(node, false, true, true);
> >   }
> > 
> > This makes the code easier to understand and removes the last remaining
> > direct use of of_reset_control_array_get(). No functional changes.
> > 
> > Fixes: f4cc91ddd856 ("usb: dwc3: of-simple: remove Amlogic GXL and AXG compatibles")
> 
> No functional change, but a Fixes: tag?  That doesn't make sense to me,
> sorry.

The referenced patch made the boolean parameters const but missed that
there is a static inline wrapper for this combination. I can drop the
Fixes: tag and describe this in the text.

regards
Philipp

Re: [PATCH] usb: dwc3-of-simple: Stop using of_reset_control_array_get() directly

[Philipp Zabel]

On Mi, 2024-01-24 at 04:39 -0800, Greg Kroah-Hartman wrote:
> On Wed, Jan 24, 2024 at 12:26:20PM +0100, Philipp Zabel wrote:
> > Use of_reset_control_array_get_optional_exclusive() instead, it is
> > implemented as:
> > 
> >   static inline struct reset_control *
> >   of_reset_control_array_get_optional_exclusive(struct device_node *node)
> >   {
> >           return of_reset_control_array_get(node, false, true, true);
> >   }
> > 
> > This makes the code easier to understand and removes the last remaining
> > direct use of of_reset_control_array_get(). No functional changes.
> 
> Does this mean the function should be removed or made static now?

I consider "hiding" it from general use by renaming it to
__of_reset_control_array_get().

There are other, indirect users of this function, but it is always
called via a more self-explanatory static inline function:

drivers/amba/bus.c:
	of_reset_control_array_get_optional_shared()

drivers/net/dsa/lantiq_gswip.c:
	of_reset_control_array_get_exclusive()

drivers/phy/cadence/phy-cadence-sierra.c:
	of_reset_control_array_get_exclusive()

drivers/phy/cadence/phy-cadence-torrent.c:
	of_reset_control_array_get_exclusive()

drivers/soc/tegra/pmc.c:
	of_reset_control_array_get_exclusive_released()

drivers/usb/dwc3/dwc3-of-simple.c:
	of_reset_control_array_get()

I would like to eventually replace the use of multiple boolean
parameters for configuration. It is hard to read and errors have
slipped through in the past (e.g. a57f68ddc886, "reset: Fix devm bulk
optional exclusive control getter").

regards
Philipp

Re: [PATCH] usb: dwc3-of-simple: Stop using of_reset_control_array_get() directly

[Greg Kroah-Hartman]

On Wed, Jan 24, 2024 at 01:56:18PM +0100, Philipp Zabel wrote:
> On Mi, 2024-01-24 at 04:39 -0800, Greg Kroah-Hartman wrote:
> > On Wed, Jan 24, 2024 at 12:26:20PM +0100, Philipp Zabel wrote:
> > > Use of_reset_control_array_get_optional_exclusive() instead, it is
> > > implemented as:
> > > 
> > >   static inline struct reset_control *
> > >   of_reset_control_array_get_optional_exclusive(struct device_node *node)
> > >   {
> > >           return of_reset_control_array_get(node, false, true, true);
> > >   }
> > > 
> > > This makes the code easier to understand and removes the last remaining
> > > direct use of of_reset_control_array_get(). No functional changes.
> > > 
> > > Fixes: f4cc91ddd856 ("usb: dwc3: of-simple: remove Amlogic GXL and AXG compatibles")
> > 
> > No functional change, but a Fixes: tag?  That doesn't make sense to me,
> > sorry.
> 
> The referenced patch made the boolean parameters const but missed that
> there is a static inline wrapper for this combination. I can drop the
> Fixes: tag and describe this in the text.

That would be best, thanks.

greg k-h

Re: [PATCH] usb: dwc3-of-simple: Stop using of_reset_control_array_get() directly

[Greg Kroah-Hartman]

On Sat, Jan 27, 2024 at 04:34:14PM -0800, Greg Kroah-Hartman wrote:
> On Wed, Jan 24, 2024 at 01:56:18PM +0100, Philipp Zabel wrote:
> > On Mi, 2024-01-24 at 04:39 -0800, Greg Kroah-Hartman wrote:
> > > On Wed, Jan 24, 2024 at 12:26:20PM +0100, Philipp Zabel wrote:
> > > > Use of_reset_control_array_get_optional_exclusive() instead, it is
> > > > implemented as:
> > > > 
> > > >   static inline struct reset_control *
> > > >   of_reset_control_array_get_optional_exclusive(struct device_node *node)
> > > >   {
> > > >           return of_reset_control_array_get(node, false, true, true);
> > > >   }
> > > > 
> > > > This makes the code easier to understand and removes the last remaining
> > > > direct use of of_reset_control_array_get(). No functional changes.
> > > > 
> > > > Fixes: f4cc91ddd856 ("usb: dwc3: of-simple: remove Amlogic GXL and AXG compatibles")
> > > 
> > > No functional change, but a Fixes: tag?  That doesn't make sense to me,
> > > sorry.
> > 
> > The referenced patch made the boolean parameters const but missed that
> > there is a static inline wrapper for this combination. I can drop the
> > Fixes: tag and describe this in the text.
> 
> That would be best, thanks.

Ah you already did so, thanks!