Re: [PATCH v3 1/1] mmc: omap_hsmmc: devm_pinctrl_get returns ERR_PTR upon error

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Andreas Fenkart <afenkart@xxxxxxxxx> [160420 04:39]:
> 2016-04-20 10:11 GMT+02:00 Dan Carpenter <dan.carpenter@xxxxxxxxxx>:
> > On Tue, Apr 19, 2016 at 03:12:27PM -0700, Arnd Bergmann wrote:
> >> On Tuesday 19 April 2016 23:04:13 Andreas Fenkart wrote:
> >> > Only the dummy implementation of devm_pinctrl_get returns NULL.
> >> > The real implementation returns ERR_PTR. By enforcing pinselect
> >> > in Kconfig we can simplify the test to check only for ERR_PTR.
> >> > detected/triggered by static code checker.
> >>
> >> This is not how the interface is meant to work:
> >>
> >> The dummy devm_pinctrl_get() intentionally returns NULL because IS_ERR()
> >> treats that as valid. All other pinctrl functions subsequently ignore
> >> that NULL pointer, so the second half of the patch is ok without the
> >> first half.
> >>
> >
> > We had that discussion.  The static checker warning triggered the
> > discussion but the real reason for this patch is that the
> > hardware will not work without PINCTRL.  So instead of compiling an
> > unusable kernel we added the select.
> 
> beaglebone needs the workaround, most chips don't. The code discussed,
> only gives some feedback to users about an incorrect configuration.
> I can drop that part or replace it with a dev_info line, pointing to
> "Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt"

Yes pinctrl is optional for sure. And leaving pinctrl out does not
produce an unusable kernel for some boards that do all the pin muxing
in the bootloader. Like Andreas says, only some boards need dynamic
pin muxing for the wake-up events.

The dev_info change sounds like papering over the issue.. I'm fine
with that, but isn't the real problem devm_pinctrl_get prototype?

> > I forget, but I think one of ARCH_OMAP2PLUS or ARCH_KEYSTONE selects
> > PINCTRL already and the other doesn't.  That was part of the discussion
> > as well.
> 
> ARCH_KEYSTONE doesn't have pinctrl, ARCH_OMAP2PLUS does

Yes.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux