linux-kernel - Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 30 Jun 2014 15:33:13 +0200
From:	Antoine Ténart 
	<antoine.tenart@...e-electrons.com>
To:	Peter Chen <peter.chen@...escale.com>
Cc:	Antoine Ténart 
	<antoine.tenart@...e-electrons.com>,
	sebastian.hesselbarth@...il.com,
	alexandre.belloni@...e-electrons.com,
	thomas.petazzoni@...e-electrons.com, zmxu@...vell.com,
	jszhang@...vell.com, linux-arm-kernel@...ts.infradead.org,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 07/12] usb: chipidea: add a generic driver

Peter,

On Fri, Jun 27, 2014 at 11:25:07AM +0800, Peter Chen wrote:
> On Tue, Jun 24, 2014 at 12:35:16PM +0200, Antoine Ténart wrote:
> >  
> >  ifneq ($(CONFIG_OF),)
> >  	obj-$(CONFIG_USB_CHIPIDEA)	+= usbmisc_imx.o ci_hdrc_imx.o
> > +	obj-$(CONFIG_USB_CHIPIDEA)	+= ci_hdrc_generic.o
> >  endif
> 
> As a generic driver, you may need to support both dt and non-dt
> solution.

Since the dt is now the best practice and since there is no need (yet)
for a non-dt usage of this driver shouldn't we let anyone needing it
implement it when the time comes?

> > +static int ci_hdrc_generic_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct ci_hdrc_generic_priv *priv;
> > +	struct ci_hdrc_platform_data ci_pdata = {
> > +		.name		= "ci_hdrc",
> 
> How about this using dev_name(&pdev->dev) for name?

Yes, why not. I don't have a strong preference.

> > +
> > +clk_err:
> > +	clk_disable_unprepare(priv->clk);
> 
> You may need to add "if (!IS_ERR(priv->clk))"

Right! I'll update this.

> > +
> > +static const struct of_device_id ci_hdrc_generic_of_match[] = {
> > +	{ .compatible = "chipidea-usb" },
> > +	{ }
> > +};
> 
> Even as a generic driver, you can also use your own compatible string.

Well, there is nothing specific about the Berlin CI. Some subsystems
use the 'generic' keyword in these cases. Do you see a particular reason
I should use some Berlin related compatible here?


Thanks for the review!

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/