All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Slawomir Stepien <sst@poczta.fm>,
	Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Cc: jic23@kernel.org, knaack.h@gmx.de, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: add driver for Microchip MCP414X/416X/424X/426X
Date: Wed, 16 Mar 2016 19:24:59 +0100	[thread overview]
Message-ID: <56E9A4FB.5060507@metafoo.de> (raw)
In-Reply-To: <20160316162544.GA6212@x220>

On 03/16/2016 05:25 PM, Slawomir Stepien wrote:
> On Mar 16, 2016 13:30, Peter Meerwald-Stadler wrote:
[...]
>> plenty of the private API, some of which seems to be debug only?
>> what is really needed to interact with a poti?
> 
> I wanted to export both the non volatile and volatile memory addresses for wiper
> position access. That is bare minimum for the poti to operate.
> 
> But I also wanted to export additional features of this chip. That is way there
> is increase and decrease API, and STATUS and TCON register access.
> 

The important part about a framework and the associated device drivers
is to expose the features of a device using a standardized interface so
you can write generic applications/libraries and share infrastructure.
If an application requires device specific knowledge to access the
features of a device you may as well write a userspace driver using i2cdev.

So when you are introducing new ABI it should at least follow the
standard naming scheme. And also try to think whether this is a feature
that is present in other similar devices and come up with a device
independent way to expose this functionality.

Let's start with the simple stuff, I don't really see the advantage of
having separate inc/dec controls. This can be handled through the
standard raw attribute. If the newly written value is one off from the
previous one use inc/dec otherwise write it directly. And even then it
might make sense to just ignore that and always write the raw value.

> The memory_map API is a way to access all the not used by chip memory addresses.
> This API I think could be deleted. But I still think that some people might find
> it useful.

This sounds more like it should maybe be exposed as a standard EEPROM
device.


  reply	other threads:[~2016-03-16 18:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-16 11:37 [PATCH] iio: add driver for Microchip MCP414X/416X/424X/426X Slawomir Stepien
2016-03-16 12:30 ` Peter Meerwald-Stadler
2016-03-16 16:25   ` Slawomir Stepien
2016-03-16 18:24     ` Lars-Peter Clausen [this message]
2016-03-16 20:02       ` Slawomir Stepien
2016-03-16 18:28     ` Daniel Baluta
2016-03-16 20:04       ` Slawomir Stepien

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=56E9A4FB.5060507@metafoo.de \
    --to=lars@metafoo.de \
    --cc=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    --cc=sst@poczta.fm \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.