On Tue, Jul 02, 2019 at 05:37:01PM +0100, Srinivas Kandagatla wrote:
> On 02/07/2019 15:44, Mark Brown wrote:
> > On Tue, Jul 02, 2019 at 09:09:16AM +0100, Srinivas Kandagatla wrote:
> > > + for (i = 0; i < ARRAY_SIZE(cpr_defaults); i++) {
> > > + regmap_bulk_write(data->regmap,
> > > + WCD934X_CODEC_CPR_WR_DATA_0,
> > > + (u8 *)&cpr_defaults[i].wr_data, 4);
> > > + regmap_bulk_write(data->regmap,
> > > + WCD934X_CODEC_CPR_WR_ADDR_0,
> > > + (u8 *)&cpr_defaults[i].wr_addr, 4);
> > What is "cpr" and should you be using a regmap patch here? Why
> > is this not with the other default updates? You've got loads of
> > random undocumented sequences like this all through the driver,
> > are they patches or are they things that should be controllable
> > by the user?
> It makes sense to have a single default map here, I will do the in next
> version. And regarding user controllable, I will go thru the list once
> again in detail and remove user controllable registers.
What is a "default map"? In terms of user controllable stuff
it's not just a question of if things are currently user
controllable but also if they should be user controllable.
> > > + return of_platform_populate(wcd->dev->of_node, NULL, NULL, wcd->dev);
> > Why are we doing this?
> I will not be using MFD in this instance as most of the resources like
> interrupts, pins, clks, SoundWire are modeled as proper drivers with
> their respective subsystems.
This is a driver for a single device, you should be able to
instantiate things without requiring binding through DT (and
hence support non-DT systems such as those using ACPI).
> This gives a advantage of reusing those drivers like SoundWire, pinctrl
> on other Qualcomm IPs as well!
> Also I did not wanted to have a custom functions or hooks in the
> drivers, so platform bus made much sense for me to use here, which can
> take care of bringing up and tearing down the devices with proper parent
> child relationship.
> This will instantiate all the child devices like pinctrl, SoundWire
> Controller and so on.
Just create platform devices like normal...
Attachment:
signature.asc
Description: PGP signature
