Messages in this thread | | | Date | Sun, 13 May 2012 13:29:46 +0100 | From | Mark Brown <> | Subject | Re: [RFC] Common clock framework for external clock generators |
| |
On Thu, May 10, 2012 at 03:11:55AM +0200, Sebastian Hesselbarh wrote:
Always CC maintainers on mails, things get lost on high volume lists.
> I recently read about the newly introduced common clock framework (ccf) > and wondered if this could be also used for external, e.g. i2c attached, > clock generators.
Yes, it should be able to be used for this. I've been posting a driver for the wm831x PMICs for quite some considerable time.
> static inline u8 si5351_reg_read(struct si5351_driver_data *data, u8 addr) > { > return (u8)i2c_smbus_read_byte_data(data->client, addr); > }
I'd suggest using regmap for the cache support (which helps with read/modify/write cycles) and because...
> static int si5351_xtal_enable(struct clk_hw *hw) > { > struct si5351_driver_data *sidata = > container_of(hw, struct si5351_driver_data, hw_xtal); > u8 reg; > > reg = si5351_reg_read(sidata, SI5351_FANOUT_ENABLE); > reg |= SI5351_XTAL_ENABLE; > si5351_reg_write(sidata, SI5351_FANOUT_ENABLE, reg); > return 0; > }
...we should be able to factor things like this out of the drivers using regmap (we've been getting some great results doing that for regulator over this release).
> static const struct clk_ops si5351_xtal_ops = { > .enable = si5351_xtal_enable, > .disable = si5351_xtal_disable,
These should be prepare() and unprepare() - enable() and disable() are called from atomic context so you can't do I2C I/O.
> static u8 si5351_clkout_get_parent(struct clk_hw *hw) > { > struct si5351_driver_data *sidata = si5351_clkout_get_data(hw); > return 0; > }
If we need empty functions like this we should fix the framework.
> sidata = kzalloc(sizeof(struct si5351_driver_data), GFP_KERNEL); > if (sidata == NULL) { > dev_err(&client->dev, "unable to allocate driver data\n"); > return -ENOMEM; > }
devm_kzalloc().
| |