On 21/10/2019 12:45, Lee Jones wrote:
On Mon, 21 Oct 2019, Srinivas Kandagatla wrote:
Thanks Lee for taking time to review.
I agree with most of the style related comments, will fix them in next version. For others I have replied it inline.
[...]
+static int wcd934x_slim_status(struct slim_device *sdev,
enum slim_device_status status)+{
- struct device *dev = &sdev->dev;
- struct wcd934x_data *wcd;
- int ret;
This is semantically odd! Why are you doing most of the initialisation and bring-up in 'status' and not 'probe'. Seems broken to me.
SLIMBus device will not be in a state to communicate before enumeration (at probe), so all the device initialization is done in status callback where it is ready for communication.
Why do we need the device to be up *before* calling probe?
To Initialize the device. And SLIMbus device registers access can not be done before enumeration.
This is same with SoundWire Bus as well!
[...]
- struct device *dev;
- struct clk *extclk;
- struct regmap *regmap;
- struct slim_device *sdev;
You don't need 'sdev' and 'dev'.
slim_device instance (sdev) is required by audio codec driver to allocate stream runtime.
You can extrapolate one from the other.
That is possible to do! I can give that a try and see in next version!