Re: [PATCH] Add ltc3562 voltage regulator driver

[Mike Looijmans]

ïOn 10/29/2014 01:30 PM, Mark Brown wrote:
> On Wed, Oct 29, 2014 at 09:16:00AM +0100, Mike Looijmans wrote:
>
> > +	if (!status->voltage_set) {
> > +		if (of_property_read_u32(dev->dev.of_node,
> > +			"ltc3562-default-voltage", &v_default) == 0) {
>
> A couple of problems here:
>
>   - This contains DT code but no DT bindings documentation; the binding
>     documentation is mandatory for any new bindings.

Should I submit a new patch to add the DT bindings and CC to the dt mailing 
list, or do you want it included in this patch? I was basically waiting for 
feedback on the driver first.

>   - It's not obvious why a "default voltage" property would be device
>     specific - what is this for and why is it being added in an
>     individual device driver?

As for the "why" part: The default is used when there isn't a consumer 
available. It'll typically be combined with "always-on". When the output is 
used for say an MMC card, the consumer will ask for a specific voltage. When 
used to regulate the IO voltage of an FPGA bank (as is the case on our board), 
there isn't a single consumer to request a particular voltage, but the min and 
max will represent the range that the IO banks support (e.g. 1v8..3v3 or 
1v2..1v8), because selecting voltages outside this range may damage the FPGA 
hardware. This allows a devicetree overlay to specify a different value for a 
different board or application, while still being able to restrict the range 
to what the hardware connected to the regulator supports.

The "default voltage" may not be device specific, but regulator.txt doesn't 
have a property that describes what we needed here, because we can't get away 
with specifying min=max as fixed-regulator does. We also cannot assume that 
default=min or default=max, because that might harm hardware connected to the 
other side of the IO bank.

I noticed after rebasing to mainline that there is now also a driver for a 
similar chip (similar in function, that is), the ltc3589, which requires 
similar properties like the resistor dividers. I think it would be good to use 
similar names in the ltc3562 driver. I'll do that first and submit a v2 patch, 
along with the DT description.

Thank you for your feedback,
Mike.



Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans@xxxxxxxx
Website: www.topic.nl

Please consider the environment before printing this e-mail

Topic zoekt gedreven (embedded) software specialisten!
http://topic.nl/vacatures/topic-zoekt-software-engineers/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/