Re: [PATCH v9 1/4] phy: qcom: m31: Fix indentation issues

[Dmitry Baryshkov]

On 22/08/2023 13:55, Dmitry Baryshkov wrote:
> On Tue, 22 Aug 2023 at 13:31, Varadarajan Narayanan
> <quic_varada@xxxxxxxxxxx> wrote:
> > * Fix indentation
> > * Drop simple success messages
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> > ---
> > v9:
> >          Fix line break alignment
> >          Remove register success print
> >          v8 version of the driver has been picked up for merge.
> >          (https://lore.kernel.org/linux-arm-msm/169226613917.81413.1200008047604336868.b4-ty@xxxxxxxxxx/)
> > v8:
> >          Change commit subject and message per review comments
> >          Don't include of_platform.h
> >          Change struct init coding style
> >          GENMASK -> BIT for one define
> > v6:
> >          Kconfig:Add COMPILE_TEST and remove USB_GADGET from 'depends'
> >                  Change 'selects' USB_PHY -> GENERIC_PHY
> >          Driver: Use correct headers
> >                  const int -> unsigned int for 'nregs' in private data
> >                  Use generic names for clk, phy in m31 phy structure
> >                  Init register details directly instead of using macro
> >                  Use dev_err_probe in the error paths of driver probe
> > v5:
> >          Kconfig and Makefile:- place snippet according to sorted order
> >          Use generic phy instead of usb-phy
> >          Use ARRAY_SIZE for reg init instead of blank last entry
> >          Fix copyright year
> >
> > v4:
> >          Remove unused enum
> >          Error handling for devm_clk_get
> > v1:
> >          Combine driver, makefile and kconfig into 1 patch
> >          Remove 'qscratch' region and its usage. The controller driver takes care
> >          of those settings
> >          Use compatible/data to handle ipq5332 init
> >          Drop the default case
> >          Get resources by index instead of name as there is only one resource
> >          Add clock
> >          Fix review comments in the driver
> > ---
> >   drivers/phy/qualcomm/phy-qcom-m31.c | 8 +++-----
> >   1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/phy/qualcomm/phy-qcom-m31.c b/drivers/phy/qualcomm/phy-qcom-m31.c
> > index ed08072..ea79c18 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-m31.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-m31.c
> > @@ -242,7 +242,7 @@ static int m31usb_phy_probe(struct platform_device *pdev)
> >          qphy->clk = devm_clk_get(dev, NULL);
> >          if (IS_ERR(qphy->clk))
> >                  return dev_err_probe(dev, PTR_ERR(qphy->clk),
> > -                                               "failed to get clk\n");
> > +                                    "failed to get clk\n");
>
> But why are you trying to fix this? aligning next line to the opening
> bracket is perfectly fine (and is one of the endorsed styles).

Ugh, excuse me. My mailer displayed alignment in a wrong way.

This LGTM except the last chunk (removal of the info message).

> >          data = of_device_get_match_data(dev);
> >          qphy->regs              = data->regs;
> > @@ -252,18 +252,16 @@ static int m31usb_phy_probe(struct platform_device *pdev)
> >          qphy->phy = devm_phy_create(dev, NULL, &m31usb_phy_gen_ops);
> >          if (IS_ERR(qphy->phy))
> >                  return dev_err_probe(dev, PTR_ERR(qphy->phy),
> > -                                               "failed to create phy\n");
> > +                                    "failed to create phy\n");
> >
> >          qphy->vreg = devm_regulator_get(dev, "vdda-phy");
> >          if (IS_ERR(qphy->vreg))
> >                  return dev_err_probe(dev, PTR_ERR(qphy->phy),
> > -                                               "failed to get vreg\n");
> > +                                    "failed to get vreg\n");
> >
> >          phy_set_drvdata(qphy->phy, qphy);
> >
> >          phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> > -       if (!IS_ERR(phy_provider))
> > -               dev_info(dev, "Registered M31 USB phy\n");
>
> This is irrelevant to the indentation.
>
> >          return PTR_ERR_OR_ZERO(phy_provider);
> >   }
> > --
> > 2.7.4

--
With best wishes
Dmitry