linux-kernel - Re: Work on iio: stating: frequency: ad9832

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: <20190401171304.00005954@huawei.com>
Date:   Mon, 1 Apr 2019 17:13:04 +0100
From:   Jonathan Cameron <jonathan.cameron@...wei.com>
To:     Marcelo Schmitt <marcelo.schmitt1@...il.com>
CC:     <linux-iio@...r.kernel.org>, <devel@...verdev.osuosl.org>,
        <linux-kernel@...r.kernel.org>, <kernel-usp@...glegroups.com>
Subject: Re: Work on iio: stating: frequency: ad9832

On Mon, 1 Apr 2019 11:25:29 -0300
Marcelo Schmitt <marcelo.schmitt1@...il.com> wrote:

> Hello,
> 
> I was looking for some work on staging: iio: ad9832 and made some
> observations while reading the driver.
> 
> Apparently it had no devicetree documentation so I tried to elaborate
> one. 
> It uses a platform_data variable to load external clock
> frequency (I tried to make it use linux's clock framework).
Good.

> Some device attributes don't seem to be standardized on
> Documentation/ABI/testing/sysfs-bus-iio and there's no specific ABI
> for ad9832 nearby nor at staging/iio/Documentation. So maybe those
> missing ABI could be documented.
Beware. It's an old driver, so it may be that we actually want to change
it's ABI rather than documenting what is there (I have haven't looked!)

> The device has to set some internal registers to operate correctly,
> AD9832_FREQXHM and AD9832_PHASEXH, would it be feasible to set iio
> chanels for this?

What are they?  If they correspond to output channels in some sensible
way then maybe...

> I couldn't understand why checkpatch.pl gave errors on IIO_DEV_ATTR_*
> macros. To me they seem to have no problem.
> Also it has that platform_data to be moved to include/linux/iio. Is
> there any special reason for it not being there already? Which are
> the criterions a platform_data need to satisfy to be put there?
A driver moving out of staging shouldn't have platform data. It needs
to be converted over to more modern mechanisms.   We don't have a problem
supporting platform data for devices that have old school device files
already in tree, but that shouldn't be the case for a driver in staging.

Hence we can clean it up and move forward with just DT bindings.
> 
> I'm sending a patchset with some things I've already done.
Cool. I'll look at those later in the week if no one beats me to them.

> 
> Is there something else that could be done in this device driver?
> Please, tell if I've forgotten something.

I'll take a look, but it may be a little while before I do.
Hopefully someone else gets there first!

Jonathan

> 
> Any advice is welcome.
> Thanks,
> 
> Marcelo