Nit: device tree convention is to use '-' rather than '_'. So this should be 
reset-gpio rather than reset_gpio.

s/_/-/ for the later property names too.


Will do, thanks

s/GPIO spec/phandle + gpio-specifier/

OK, Same here



+  - chgfreq    : Charge Pump Frequency values 0x00-0x0F

In general, longer but more easily understood names are preferred (e.g.
"clock-frequency"). This could be "charge-pump-frequency".

As Mark Brown pointed out, I would prefer to leave this as a reference to a register in the device. In general I was hoping to keep the original platform data names as carried over to dt.

When you say "values 0x00-0x0F" you mean the value is in that range?
Inclusive?

I'll be more specific, Thanks

Due to punctuation, upon first reading I read this as being an array of values. 
 It would be nice to have some clarification to prevent others maknig the smae 
mistake.

+  - mica_sel   : MIC A single ended input select MIC1/MIC2
+  - micb_sel   : MIC B single ended input select MIC1/MIC2

Is this a boolean? What values are expected?

Could you elaborate on what this describes?

+  - mica_cfg   : MIC A single-ended or differential select
+  - micb_cfg   : MIC A single-ended or differential select

Could you elaborate on what these describe, what values are expected, etc?


I will explain values better, Thanks


+  - micbias_lvl: Set the output voltage level on the MICBIAS Pin
+                0x00 = 0.5 x VA
+                0x01 = 0.6 x VA
+                0x02 = 0.7 x VA
+                0x03 = 0.8 x VA
+                0x04 = 0.83 x VA
+                0x05 = 0.91 x VA

Is this not something we'd want to change at runtime instead? Why does this 
need to be in the dt?


This is set based on the system design so it usually will be a one time setting at boot.

I'll send a v2 shortly,

Thank you for the review and tips for the next ones.

Regards,
Brian

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to