moikka!
On 09/06/2014 09:09 AM, Akihiro TSUKADA wrote:
Moi!
Yes, using the I2C binding way provides a better decoupling than using the
legacy way. The current dvb_attach() macros are hacks that were created
by the time where the I2C standard bind didn't work with DVB.
I understand. I converted my code to use i2c binding model,
but I'm uncertain about a few things.
1. How to load the modules of i2c driver?
Currently I use request_module()/module_put()
like an example (ddbrige-core.c) from Antti does,
but I'd prefer implicit module loading/ref-counting
like in dvb_attach() if it exists.
Maybe I haven't found the best way yet, but that was implementation I
made for AF9035 driver:
https://patchwork.linuxtv.org/patch/25764/
Basically it is 2 functions, af9035_add_i2c_dev() and af9035_del_i2c_dev()
2. Is there a standard way to pass around dvb_frontend*, i2c_client*,
regmap* between bridge(dvb_adapter) and demod/tuner drivers?
Currently I use config structure for the purpose, which is set to
dev.platform_data (via i2c_board_info/i2c_new_device()) or
dev.driver_data (via i2c_{get,set}_clientdata()),
but using config as both IN/OUT looks a bit hacky.
In my understanding, platform_data is place to pass environment specific
config to driver. get/set client_data() is aimed to carry pointer for
device specific instance "state" inside a driver. Is it possible to set
I2C client data before calling i2c_new_device() and pass pointer to driver?
There is also IOCTL style command for I2C, but it is legacy and should
not be used.
Documentation/i2c/busses/i2c-ocores
Yet, using config to OUT seems to be bit hacky for my eyes too. I though
replacing all OUT with ops when converted af9033 driver. Currently
caller fills struct af9033_ops and then af9033 I2C probe populates ops.
See that patch:
https://patchwork.linuxtv.org/patch/25746/
Does this kind of ops sounds any better?
EXPORT_SYMBOL() is easiest way to offer outputs, like
EXPORT_SYMBOL(get_frontend), EXPORT_SYMBOL(get_i2c_adapter). But we want
avoid those exported symbols.
3. Should I also use RegMap API for register access?
I tried using it but gave up,
because it does not fit well to one of my use-case,
where (only) reads must be done via 0xfb register, like
READ(reg, buf, len) -> [addr/w, 0xfb, reg], [addr/r, buf[0]...],
WRITE(reg, buf, len) -> [addr/w, reg, buf[0]...],
and regmap looked to me overkill for 8bit-reg, 8bit-val cases
and did not simplify the code.
so I'd like to go without RegMap if possible,
since I'm already puzzled enough by I2C binding, regmap, clock source,
as well as dvb-core, PCI ;)
I prefer RegMap API, but I am only one who has started using it yet. And
I haven't converted any demod driver having I2C bus/gate/repeater for
tuner to that API. It is because of I2C locking with I2C mux adapter,
you need use unlocked version of i2c_transfer() for I2C mux as I2C bus
lock is already taken. RegMap API does not support that, but I think it
should be possible if you implement own xfer callback for regmap. For RF
tuners RegMap API should be trivial and it will reduce ~100 LOC from driver.
But if you decide avoid RegMap API, I ask you add least implementing
those I2C read / write function parameters similarly than RegMap API
does. Defining all kind of weird register write / read functions makes
life harder. I converted recently IT913x driver to RegMap API and
biggest pain was there very different register read / write routines. So
I need to do a lot of work in order convert functions first some common
style and then it was trivial to change RegMap API.
https://patchwork.linuxtv.org/patch/25766/
https://patchwork.linuxtv.org/patch/25757/
I quickly overlooked that demod driver and one which looked crazy was
LNA stuff. You implement set_lna callback in demod, but it is then
passed back to PCI driver using frontend callback. Is there some reason
you hooked it via demod? You could implement set_lna in PCI driver too.
regards
Antti
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
