linux-kernel - Re: [PATCH 1/4] ath10k: snoc: remove 'wcn3990' from generic resource handling

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:   Tue, 16 Oct 2018 16:47:05 -0700
From:   Brian Norris <briannorris@...omium.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Kalle Valo <kvalo@....qualcomm.com>, ath10k@...ts.infradead.org,
        linux-wireless@...r.kernel.org, govinds@...eaurora.org,
        Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] ath10k: snoc: remove 'wcn3990' from generic resource handling

On Tue, Oct 16, 2018 at 4:43 PM Doug Anderson <dianders@...omium.org> wrote:
> On Fri, Oct 12, 2018 at 5:55 PM Brian Norris <briannorris@...omium.org> wrote:
> >
> > ath10k_wcn3990_clk_info and ath10k_wcn3990_vreg_info are not
> > WCN3990-specific structures. They hold generic data. So don't name them
> > with wcn3990 specifics.
> >

> > -static struct ath10k_wcn3990_vreg_info vreg_cfg[] = {
> > +static struct ath10k_vreg_info vreg_cfg[] = {
>
> Ironically, you could sorta make the argument that this should be:
>
> static struct ath10k_vreg_info wcn3990_vreg_cfg

Hehe, good point.

> AKA the "wcn3990" shouldn't be in the name of the structure (since all
> snoc devices can have the concept of an array of regulators) but
> wcn3990 could be in the name of the variable since it's possible that
> different snoc devices could have different arrays.  However I'm OK w/
> waiting to do that part until we actually see a different snoc device
> with a different array.

But I think that is a pretty reasonable conclusion.

There's still some other strange stuff in this driver too, like the
fact that this is NOT a const array -- we assign things dynamically to
the regulator fields within the array -- that we would probably want
to fix if this is really supposed to be a generic multi-IP driver.

> Reviewed-by: Douglas Anderson <dianders@...omium.org>

Thanks,
Brian