On Tue, Dec 08, 2015 at 07:19:05AM +0100, Sascha Hauer wrote:
> > + hub_data = devm_kzalloc(dev, sizeof(*hub_data), GFP_KERNEL);
> > + if (!hub_data)
> > + return -ENOMEM;
> > +
> > + if (dev->of_node) {
> > + struct device_node *node = dev->of_node;
> > +
> > + hub_data->clk = devm_clk_get(dev, "external_clk");
>
> Please drop such _clk suffixes. The context already makes it clear that
> it's a clock.
>
Will change.
> > + if (IS_ERR(hub_data->clk)) {
> > + dev_dbg(dev, "Can't get external clock: %ld\n",
> > + PTR_ERR(hub_data->clk));
> > + }
> > +
> > + /*
> > + * Try to get the information for HUB reset, the
> > + * default setting like below:
> > + *
> > + * - Reset state is low
> > + * - The duration is 50us
> > + */
> > + if (of_find_property(node, "hub-reset-active-high", NULL))
> > + reset_pol = 1;
> > +
> > + of_property_read_u32(node, "hub-reset-duration-us",
> > + &duration_us);
> > +
> > + gpiod_reset = devm_gpiod_get_optional(dev, "hub-reset",
> > + reset_pol ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> > + ret = PTR_ERR_OR_ZERO(gpiod_reset);
> > + if (ret) {
> > + dev_err(dev, "Failed to get reset gpio, err = %d\n",
> > + ret);
> > + return ret;
> > + }
> > + } else if (pdata) {
> > + hub_data->clk = pdata->ext_clk;
>
> Passing clocks in platform_data is a no go. clocks must always be
> retrieved with clk_get.
Yes, you are right.
>
> > + duration_us = pdata->gpio_reset_duration_us;
> > + reset_pol = pdata->gpio_reset_polarity;
> > +
> > + if (gpio_is_valid(pdata->gpio_reset)) {
> > + ret = devm_gpio_request_one(
> > + dev, pdata->gpio_reset, reset_pol
> > + ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> > + dev_name(dev));
> > + if (!ret)
> > + gpiod_reset = gpio_to_desc(pdata->gpio_reset);
>
> If the gpio is valid then being unable to get it is an error.
I can't catch you, if the gpio is invalid, its descriptor will be NULL,
then, the code will not do any gpio operation.
>
> Do you need this platform_data stuff at all?
>
If not, how can I cover the platform which does not use dts, but still
uses these kinds of HUBs?
--
Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
