On Fri, Jun 14, 2019 at 10:53:13PM +0530, Manikanta Maddireddy wrote:
>
>
> On 14-Jun-19 10:23 PM, Lorenzo Pieralisi wrote:
> > On Fri, Jun 14, 2019 at 10:00:49PM +0530, Manikanta Maddireddy wrote:
> >
> > [...]
> >
> >> GPIO based PERST# is per-platform requirement.
> >> If DT prop is not present, then devm_gpiod_get_from_of_node() returns
> >> NULL gpio_desc.
> >>
> >> struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
> >> const char *propname, int index,
> >> enum gpiod_flags dflags,
> >> const char *label)
> >> {
> >> struct gpio_desc *desc;
> >> unsigned long lflags = 0;
> >> enum of_gpio_flags flags;
> >> bool active_low = false;
> >> bool single_ended = false;
> >> bool open_drain = false;
> >> bool transitory = false;
> >> int ret;
> >>
> >> desc = of_get_named_gpiod_flags(node, propname,
> >> index, &flags);
> >>
> >> if (!desc || IS_ERR(desc)) {
> >> */* If it is not there, just return NULL */****if (PTR_ERR(desc) == -ENOENT)****return NULL;*
> >> return desc;
> >> }
> >> ...
> >>
> >> }
> > Ok. My point then is that you have no way to enforce this requirement on
> > platforms that actually need it, I do not even know if there is a
> > way you can do it (I was thinking along the lines of using a
> > compatible string to detect whether the GPIO #PERST reset is mandatory)
> > but maybe this is not even a SOC property.
> >
> > Maybe what I am asking is overkill, I just wanted to understand.
> >
> > I was just asking a question to understand how you handle the case
> > where a GPIO pin definition is missing in DT for a platform that
> > actually needs it, the driver will probe but nothing will work.
> >
> > It would be good to describe this and capture it in the commit log.
> >
> > Thanks,
> > Lorenzo
>
> I can't think of a easy way to enforce this requirement. As you said
> compatible string is per SOC, so we can't use it for a platform.
> This issue is present on only one platform, so it is hard to miss the
> DT property. That is the reason for publishing this patch with out this
> enforcement in driver.
>
> I thought for changing PERST# to GPIO for all platform, but testing is
> a tedious job. Also I don't have Tegra20 and Tegra30 platforms.
Yeah, let's not go that way. The standard way to do this is to use the
SFIO and let the PCIe controller and driver handle this. It's working
just fine on all platforms currently supported upstream. Using direct
GPIO for PERST# is a workaround, so let's not proliferate unless it is
absolutely necessary.
With an updated commit message, this is:
Acked-by: Thierry Reding <treding@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
