| Message ID | 1415025649-8119-3-git-send-email-javier.martinez@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On pon, 2014-11-03 at 15:40 +0100, Javier Martinez Canillas wrote: > The regulator-initial-mode and regulator-mode DT properties allows to > configure the regulator operating modes at startup or when a system > enters into a susend state. > > But these properties use as valid values the operating modes supported > by each device while the core deals with the standard operating modes. > So a mapping function is needed to translate from the hardware specific > modes to the standard ones. > > This mapping is a non-varying configuration for each regulator, so add > a function pointer to struct regulator_desc that will allow drivers to > define their callback to do the modes translation. > > Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > --- > include/linux/regulator/driver.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h > index 28da08e..b54d037 100644 > --- a/include/linux/regulator/driver.h > +++ b/include/linux/regulator/driver.h > @@ -243,6 +243,8 @@ enum regulator_type { > * > * @enable_time: Time taken for initial enable of regulator (in uS). > * @off_on_delay: guard time (in uS), before re-enabling a regulator > + * > + * @map_modes: Callback invoked to translate between hardware to standard modes. Initially I thought it should map from standard to hardware. But then I looked at max77802 implementation and it maps from hardware to standard. Anyway I got confused (both are "modes" and both unsigned ints). Could you describe which should be returned? > */ > struct regulator_desc { > const char *name; > @@ -285,6 +287,8 @@ struct regulator_desc { > unsigned int enable_time; > > unsigned int off_on_delay; > + > + unsigned int (*map_modes)(unsigned int mode); Shouldn't this be in regulator ops? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Krzysztof, Thanks a for your feedback. On 11/04/2014 11:31 AM, Krzysztof Kozlowski wrote: >> + * >> + * @map_modes: Callback invoked to translate between hardware to standard modes. > > Initially I thought it should map from standard to hardware. But then I > looked at max77802 implementation and it maps from hardware to standard. > Anyway I got confused (both are "modes" and both unsigned ints). > > Could you describe which should be returned? > Sure, maybe rewording to: "Callback invoked to translate from hardware to standard modes." ? But I'll add also document that the parameter should be a hardware mode and the return value a standard mode. >> */ >> struct regulator_desc { >> const char *name; >> @@ -285,6 +287,8 @@ struct regulator_desc { >> unsigned int enable_time; >> >> unsigned int off_on_delay; >> + >> + unsigned int (*map_modes)(unsigned int mode); > > Shouldn't this be in regulator ops? > regulator ops are for the operations that a regulator support (enable, disable, set mode, etc). All the thse are actions but how to translate between hardware and standard modes is not an action but a non-varying configuration of the regulator. So I believe that regulator desc was what fit the most. I don't have a strong opinion though if people think that it should be in regulator ops instead. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On wto, 2014-11-04 at 12:02 +0100, Javier Martinez Canillas wrote: > Hello Krzysztof, > > Thanks a for your feedback. > > On 11/04/2014 11:31 AM, Krzysztof Kozlowski wrote: > >> + * > >> + * @map_modes: Callback invoked to translate between hardware to standard modes. > > > > Initially I thought it should map from standard to hardware. But then I > > looked at max77802 implementation and it maps from hardware to standard. > > Anyway I got confused (both are "modes" and both unsigned ints). > > > > Could you describe which should be returned? > > > > Sure, maybe rewording to: > > "Callback invoked to translate from hardware to standard modes." ? > > But I'll add also document that the parameter should be a hardware > mode and the return value a standard mode. Great! > > >> */ > >> struct regulator_desc { > >> const char *name; > >> @@ -285,6 +287,8 @@ struct regulator_desc { > >> unsigned int enable_time; > >> > >> unsigned int off_on_delay; > >> + > >> + unsigned int (*map_modes)(unsigned int mode); > > > > Shouldn't this be in regulator ops? > > > > regulator ops are for the operations that a regulator support > (enable, disable, set mode, etc). All the thse are actions but > how to translate between hardware and standard modes is not an > action but a non-varying configuration of the regulator. > > So I believe that regulator desc was what fit the most. I don't > have a strong opinion though if people think that it should be > in regulator ops instead. I understand, it's fine for me. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h index 28da08e..b54d037 100644 --- a/include/linux/regulator/driver.h +++ b/include/linux/regulator/driver.h @@ -243,6 +243,8 @@ enum regulator_type { * * @enable_time: Time taken for initial enable of regulator (in uS). * @off_on_delay: guard time (in uS), before re-enabling a regulator + * + * @map_modes: Callback invoked to translate between hardware to standard modes. */ struct regulator_desc { const char *name; @@ -285,6 +287,8 @@ struct regulator_desc { unsigned int enable_time; unsigned int off_on_delay; + + unsigned int (*map_modes)(unsigned int mode); }; /**
The regulator-initial-mode and regulator-mode DT properties allows to configure the regulator operating modes at startup or when a system enters into a susend state. But these properties use as valid values the operating modes supported by each device while the core deals with the standard operating modes. So a mapping function is needed to translate from the hardware specific modes to the standard ones. This mapping is a non-varying configuration for each regulator, so add a function pointer to struct regulator_desc that will allow drivers to define their callback to do the modes translation. Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> --- include/linux/regulator/driver.h | 4 ++++ 1 file changed, 4 insertions(+)