Re: [PATCH v4] usb: chipidea: removing of_find_property

Peter Chen
2015-11-18T07:27:30Z

On Wed, Nov 18, 2015 at 11:48:36AM +0530, Saurabh Sengar wrote:
> On 18 November 2015 at 11:35, Peter Chen <peter.chen@freescale.com> wrote:
> > On Wed, Nov 18, 2015 at 09:40:12AM +0530, Saurabh Sengar wrote:
> >> call to of_find_property() before of_property_read_u32() is unnecessary.
> >> of_property_read_u32() anyway calls to of_find_property() only.
> >>
> >> Signed-off-by: Saurabh Sengar <saurabh.truth@gmail.com>
> >> ---
> >> v4 : removed return type check for optional property 'itc-setting'
> >>
> >> drivers/usb/chipidea/core.c | 57 +++++++++++++++++----------------------------
> >> 1 file changed, 22 insertions(+), 35 deletions(-)
> >>
> >> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> >> index 965d0e2..3d1c3c5 100644
> >> --- a/drivers/usb/chipidea/core.c
> >> +++ b/drivers/usb/chipidea/core.c
> >> @@ -688,52 +688,39 @@ static int ci_get_platdata(struct device *dev,
> >> if (usb_get_maximum_speed(dev) == USB_SPEED_FULL)
> >> platdata->flags |= CI_HDRC_FORCE_FULLSPEED;
> >>
> >> - if (of_find_property(dev->of_node, "phy-clkgate-delay-us", NULL))
> >> - of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
> >> + of_property_read_u32(dev->of_node, "phy-clkgate-delay-us",
> >> &platdata->phy_clkgate_delay_us);
> >>
> >> platdata->itc_setting = 1;
> >> - if (of_find_property(dev->of_node, "itc-setting", NULL)) {
> >> - ret = of_property_read_u32(dev->of_node, "itc-setting",
> >> - &platdata->itc_setting);
> >> - if (ret) {
> >> - dev_err(dev,
> >> - "failed to get itc-setting\n");
> >> - return ret;
> >> - }
> >> - }
> >>
> >> - if (of_find_property(dev->of_node, "ahb-burst-config", NULL)) {
> >> - ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
> >> - &platdata->ahb_burst_config);
> >> - if (ret) {
> >> - dev_err(dev,
> >> - "failed to get ahb-burst-config\n");
> >> - return ret;
> >> - }
> >> + of_property_read_u32(dev->of_node, "itc-setting",
> >> + &platdata->itc_setting);
> >> +
> >> + ret = of_property_read_u32(dev->of_node, "ahb-burst-config",
> >> + &platdata->ahb_burst_config);
> >> + if (!ret) {
> >> platdata->flags |= CI_HDRC_OVERRIDE_AHB_BURST;
> >> + } else if (ret != -EINVAL) {
> >> + dev_err(dev, "failed to get ahb-burst-config\n");
> >> + return ret;
> >> }
>
> >Sorry, one more comment, why we don't quit if the 'ret' is other error
> >value?
> >
> > Peter
>
> We quit if error is anything other then -EINVAL.
> In case of -EINVAL, it means we are deliberating ignoring that
> property thus left it.
> Also the previous functionality was like this when we were using
> of_find_property().
> Please let me know if this need to be changed, that we need to return
> in all kind of error, I will fix it and send it in patch v5.
>

I am clear now, I will queue it.

--

Best Regards,
Peter Chen