From: Jonathan Cameron <jonathan.cameron@huawei.com>
To: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Cc: <linux-iio@vger.kernel.org>, <devel@driverdev.osuosl.org>,
<linux-kernel@vger.kernel.org>, <kernel-usp@googlegroups.com>
Subject: Re: Work on iio: stating: frequency: ad9832
Date: Mon, 1 Apr 2019 17:13:04 +0100 [thread overview]
Message-ID: <20190401171304.00005954@huawei.com> (raw)
In-Reply-To: <20190401142529.xgkqmndinctiro3l@smtp.gmail.com>
On Mon, 1 Apr 2019 11:25:29 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.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
next prev parent reply other threads:[~2019-04-01 16:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-01 14:25 Work on iio: stating: frequency: ad9832 Marcelo Schmitt
2019-04-01 16:13 ` Jonathan Cameron [this message]
2019-04-02 7:12 ` Alexandru Ardelean
2019-04-02 16:27 ` Marcelo Schmitt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190401171304.00005954@huawei.com \
--to=jonathan.cameron@huawei.com \
--cc=devel@driverdev.osuosl.org \
--cc=kernel-usp@googlegroups.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcelo.schmitt1@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).