linux-kernel - Re: [PATCH 01/14] dmaengine: dma-jz4780: Avoid hardcoding number of channels

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>] [day] [month] [year] [list]

Date:   Wed, 11 Jul 2018 17:44:03 +0530
From:   Vinod <vkoul@...nel.org>
To:     Paul Cercueil <paul@...pouillou.net>,
        Rob Herring <robh+dt@...nel.org>
Cc:     Mark Rutland <mark.rutland@....com>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Paul Burton <paul.burton@...s.com>,
        James Hogan <jhogan@...nel.org>,
        Zubair Lutfullah Kakakhel <Zubair.Kakakhel@...tec.com>,
        Mathieu Malaterre <malat@...ian.org>,
        Daniel Silsby <dansilsby@...il.com>, dmaengine@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mips@...ux-mips.org
Subject: Re: [PATCH 01/14] dmaengine: dma-jz4780: Avoid hardcoding number of
 channels

Hi Paul,

On 10-07-18, 17:29, Paul Cercueil wrote:
> > >  +static const unsigned int jz4780_dma_nb_channels[] = {
> > >  +	[ID_JZ4780] = 32,
> > >  +};
> > >  +
> > >  +static const struct of_device_id jz4780_dma_dt_match[] = {
> > >  +	{ .compatible = "ingenic,jz4780-dma", .data = (void *)ID_JZ4780 },
> > >  +	{},
> > >  +};
> > 
> > Looking at description I was hoping that channels would be in DT,
> > channels is hardware information, so should come from DT rather than
> > coding the kernel...
> 
> I had a talk with Linus Walleij (GPIO maintainer) about that:
> http://lkml.iu.edu/hypermail/linux/kernel/1701.3/05422.html
> 
> And I agree with him, we shouldn't have in devicetree what we can deduce
> from the compatible string. But there doesn't seem to be an enforced
> policy about it.

Looking at this, yes that can be done as you have implemented but adding
new compatible and tables every time seems not so great to me.

If DT can describe these hardware features then driver can take action generically
and we avoid these tables and skip some patches here..

> 
> @Rob, what do you think?

Rob what is the recommendation here?

> 
> > >  -	jzdma = devm_kzalloc(dev, sizeof(*jzdma), GFP_KERNEL);
> > >  +	if (of_id)
> > >  +		version = (enum jz_version)of_id->data;
> > >  +	else
> > >  +		version = ID_JZ4780; /* Default when not probed from DT */
> > 
> > where else would it be probed from.... ?
> 
> Platform, MFD driver, etc. But not likely to happen.
> I can remove these lines if you want.

Lets add when we land support for those.

-- 
~Vinod