linux-kernel - Re: [PATCH v3 2/6] Add the main bulk of core driver for SI476x code

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: <5089BC7A.80103@convergeddevices.net>
Date:	Thu, 25 Oct 2012 15:26:02 -0700
From:	Andrey Smirnov <andrey.smirnov@...vergeddevices.net>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
CC:	<hverkuil@...all.nl>, <mchehab@...hat.com>,
	<sameo@...ux.intel.com>, <perex@...ex.cz>, <tiwai@...e.de>,
	<linux-media@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/6] Add the main bulk of core driver for SI476x code

On 10/25/2012 12:45 PM, Mark Brown wrote:
> On Tue, Oct 23, 2012 at 11:44:28AM -0700, Andrey Smirnov wrote:
>
>> +	core->regmap = devm_regmap_init_si476x(core);
>> +	if (IS_ERR(core->regmap)) {
> This really makes little sense to me, why are you doing this?  Does the
> device *really* layer a byte stream on top of I2C for sending messages
> that look like marshalled register reads and writes?
The SI476x chips has a concept of a "property". Each property having
16-bit address and 16-bit value. At least a portion of a chip
configuration is done by modifying those properties. In order to
manipulate those properties user is expected to issue what is called a
"command". For manipulating "properties" there are two "commands":
- SET_PROPERTY which is I2C write of 6 bytes of the following layout:
    | 0x13 | 0x00 | Address High Byte | Address Low Byte | Property Data
High Byte | Property Data Low Byte |
    After the command is finished being executed the 1 byte read would
contain the status byte

    followed by 1 byte read which contain status byte
- GET_PROPERTY which is I2C write of 4 bytes of the following layout:
    | 0x13 | 0x00 | Address High Byte | Address Low Byte |
    After the command is finished being executed the 4 byte read would
have the following layout:
    | Status Byte | Reserved Byte | Property Value High Byte | Property
Value Low Byte |

The chip does not operate continuously in the AM/FM frequency range,
instead the user is expected to power-down/power-up the chip in a
certain "mode" which in my case can be either AM or FM tuner. There are
two ways of doing that one is to send a power down command to the
device, the second one is to toggle reset line of the chip. Both methods
will reset the values of the aforementioned "properties". Because V4L2
user-space interface presents a tuner as the one continuously operating
in the whole AM/FM range it is necessary for me to do those AM/FM
switches transparently when handling tuning or seeking requests from the
user. That means that I need to cache the values of the properties I
care about in the driver and restore them when user switches to the
mode(for example when AM->FM->AM transition happens)

The other quirk of that chip is that some properties are only accessible
in certain modes(for example it is impossible to configure RDS interrupt
sources, which is FM specific, in AM mode), but some of the controls I
expose to user-land change the values of the properties and since AM/FM
switches happen transparently to the user in the situation when FM
specific property is changed while tuner is in AM mode, the driver has
to cache the value and write it when the switch to FM would take place.

Also due to the way the driver uses the chip it is only powered up when
the corresponding file in devfs(e.g. /dev/radio0) is opened at least by
one user which means that unless there is a user who opened the file all
the SET/GET_PROPERTY commands sent to it will be lost. The codec driver
for that chip does not have any say in the power management policy(while
all the audio configuration is done via "properties") if the chip is not
powered up the driver has to cache the configuration values it has so
that they can be applied later.

So, since I have to implement a caching functionality in the driver, in
order to avoid reinventing the wheel I opted for using 'regmap' API for
this.
Of course, It is possible that I misunderstood the purpose and
capabilities of the 'regmap' framework, which would make my code look
very silly indeed. If that is the case I'll just re-implement it using
some sort of ad-hoc version of caching.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/