On 02/07/2019 17:57, Mark Brown wrote:
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.
I meant default updates here. These magic values and list came from
downstream android code, so I will have to audit them before I can say
that it will be useful for them to be exposed to user.
Most of things that made sense to provide a user control for the
usecases of playback/capture/sidetone/speakers are already exposed via
mixer controls.
+ 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).
My view point atleast from hw side was that Codec is Parent which is
encompassing these different blocks and bus interface. DT representation
clearly showed that relationship between the parent and child devices.
Binding it through DT will make sure that resources are ready before
they are instantiated.
All the child devices also have some machine/board specific properties
and dependency on resources from parent like regmap, clks, and bus.
In ACPI case, am not 100% sure how these will be represented inline with
hw representation.
Are you suggesting to use MFD here?
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...
These are already modeled as platform devices, except the fact that
these are children of Codec device.
thanks,
srini
