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