linux-kernel - Re: [PATCH v6 4/8] iio: dac: adi-axi-dac: extend features

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20241019160817.10c3a2bf@jic23-huawei>
Date: Sat, 19 Oct 2024 16:08:17 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: Angelo Dureghello <adureghello@...libre.com>, David Lechner 
 <dlechner@...libre.com>, Nuno Sá <nuno.sa@...log.com>,
 Lars-Peter Clausen  <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski  <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Olivier Moysan  <olivier.moysan@...s.st.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Mark Brown 
 <broonie@...nel.org>
Subject: Re: [PATCH v6 4/8] iio: dac: adi-axi-dac: extend features


> > > >  static int axi_dac_probe(struct platform_device *pdev)
> > > >  {
> > > > -	const unsigned int *expected_ver;
> > > >  	struct axi_dac_state *st;
> > > >  	void __iomem *base;
> > > >  	unsigned int ver;
> > > > @@ -566,15 +793,26 @@ static int axi_dac_probe(struct platform_device *pdev)
> > > >  	if (!st)
> > > >  		return -ENOMEM;
> > > >  
> > > > -	expected_ver = device_get_match_data(&pdev->dev);
> > > > -	if (!expected_ver)
> > > > +	st->info = device_get_match_data(&pdev->dev);
> > > > +	if (!st->info)
> > > >  		return -ENODEV;
> > > >  
> > > > -	clk = devm_clk_get_enabled(&pdev->dev, NULL);
> > > > +	clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");  
> > > 
> > > This will break existing users that don't have clock-names
> > > in the DT. It should be fine to leave it as NULL in which
> > > case it will get the clock at index 0 in the clocks array
> > > even if there is more than one clock.
> > >   
> > 
> > mm, are there existing users except this hs driver right now ?
> > 
> > Clock names are actually described in the example, and if missing,
> > also retrieving "dac_clk" would fail.
> >   
> 
> There's already a frontend DAC using the generic DAC implementation. So, in theory,
> yes... We can already have users not setting clock-names in DT that would now fail to
> probe with this patch. David is only suggesting leaving this call to NULL. For
> dac_clk we do need the *id in devm_clk_get_enabled().
> 
> Maybe it would also be worth mentioning in the bindings that s_axi_aclk needs to be
> the first entry in clocks and clock-names for backward compatibility.

Usual trick for this is match on clk name first and then fallback to no name
to pick up old DT that didn't set clk names.

That way should be no need constrain the order when it is specified.
Slight hicup is new DT, old kernel. In which case maybe the wrong clock is started
but I think you only have the multiple clocks for new cases so this should be fine.

Just sprinkle some comments alongside the fallback code to say why it is there.


Jonathan

> 
> - Nuno Sá
> > >   
>