| Message ID | 1412775847-15213-2-git-send-email-javier.martinez@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 08, 2014 at 03:44:03PM +0200, Javier Martinez Canillas wrote: > But currently there isn't a way to do the same with DeviceTrees. Argubly > the operating modes are Linux-specific so that information should not be > in the DT which should be used to only describe hardware. But regulators > having different operating modes is also a hardware property since many > PMICs have support to set different modes for their regulators. That doesn't mean that the definition of those modes is something we can sensibly provide in generic code, especially in a completely undocumented fashion (perhaps you've done that later in the patch series but bisection also applies to reviewability).
Hello Mark, Thanks for the feedback. On 10/08/2014 04:25 PM, Mark Brown wrote: > On Wed, Oct 08, 2014 at 03:44:03PM +0200, Javier Martinez Canillas wrote: > >> But currently there isn't a way to do the same with DeviceTrees. Argubly >> the operating modes are Linux-specific so that information should not be >> in the DT which should be used to only describe hardware. But regulators >> having different operating modes is also a hardware property since many >> PMICs have support to set different modes for their regulators. > > That doesn't mean that the definition of those modes is something we can > sensibly provide in generic code, especially in a completely > undocumented fashion (perhaps you've done that later in the patch series > but bisection also applies to reviewability). > Yes, patch #3 updates the regulator DT binding doc and documents what each regulator mode is supposed to be. Basically is just a short description of what is already documented in linux/regulator/consumer.h [0]. If what is enough for you I can reorganize the patch-set so that patch is the first one. As a general question, now that the convention is for DT binding docs to go in a separate patch, should the DT documentation be added before or after that code using these bindings is added? That is something that is not explained in Documentation/devicetree/bindings/submitting-patches.txt. Best regards, Javier [0]: http://lxr.free-electrons.com/source/include/linux/regulator/consumer.h#L40 -- 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 Wed, Oct 08, 2014 at 04:38:53PM +0200, Javier Martinez Canillas wrote: > On 10/08/2014 04:25 PM, Mark Brown wrote: > > That doesn't mean that the definition of those modes is something we can > > sensibly provide in generic code, especially in a completely > > undocumented fashion (perhaps you've done that later in the patch series > > but bisection also applies to reviewability). > As a general question, now that the convention is for DT binding docs to go > in a separate patch, should the DT documentation be added before or after > that code using these bindings is added? It fairly obviously needs to go before so that reviewers can tell if the code is actually implementing the binding.
On 10/08/2014 05:12 PM, Mark Brown wrote: > On Wed, Oct 08, 2014 at 04:38:53PM +0200, Javier Martinez Canillas wrote: >> On 10/08/2014 04:25 PM, Mark Brown wrote: > >> > That doesn't mean that the definition of those modes is something we can >> > sensibly provide in generic code, especially in a completely >> > undocumented fashion (perhaps you've done that later in the patch series >> > but bisection also applies to reviewability). > >> As a general question, now that the convention is for DT binding docs to go >> in a separate patch, should the DT documentation be added before or after >> that code using these bindings is added? > > It fairly obviously needs to go before so that reviewers can tell if the > code is actually implementing the binding. > Well, is not fairly obvious to me. One can also say the opposite, why the kernel is documenting a DT binding that is not (currently) implemented? That's why what makes the most sense for me is what the old convention did, add the DT binding docs in the same patch that implements the binding. Anyways, thanks for letting me know what is the convention today. -- 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 Wed, Oct 08, 2014 at 05:29:26PM +0100, Javier Martinez Canillas wrote: > On 10/08/2014 05:12 PM, Mark Brown wrote: > > On Wed, Oct 08, 2014 at 04:38:53PM +0200, Javier Martinez Canillas wrote: > >> On 10/08/2014 04:25 PM, Mark Brown wrote: > > > >> > That doesn't mean that the definition of those modes is something we can > >> > sensibly provide in generic code, especially in a completely > >> > undocumented fashion (perhaps you've done that later in the patch series > >> > but bisection also applies to reviewability). > > > >> As a general question, now that the convention is for DT binding docs to go > >> in a separate patch, should the DT documentation be added before or after > >> that code using these bindings is added? > > > > It fairly obviously needs to go before so that reviewers can tell if the > > code is actually implementing the binding. > > > > Well, is not fairly obvious to me. One can also say the opposite, why the > kernel is documenting a DT binding that is not (currently) implemented? Checkpatch will complain regarding undocumented bindings, so from a pragmatic point of view the binding must come first. Personally, when I read a patch series I do an initial pass in-order, and having the binding first makes things clearer. I might have some questions regarding the binding that the driver answers later, and it makes it easier to spot undocumented properties or conventions used by the driver. Doing so the other way around usually leaves me with more questions at the end. > That's why what makes the most sense for me is what the old convention did, > add the DT binding docs in the same patch that implements the binding. Having a separate patch for the binding is very helpful for those of us doing review. For one thing it helps us to find the binding document, which can be important when a driver is thousands of lines long. For another it means that we can be clear that our Acked-by, Reviewed-by, etc apply to the binding and not necessarily the rest of the code. For small patches, this is obviously less of a concern. Thanks, Mark. -- 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 Mark, On 10/09/2014 12:27 PM, Mark Rutland wrote: >> >> Well, is not fairly obvious to me. One can also say the opposite, why the >> kernel is documenting a DT binding that is not (currently) implemented? > > Checkpatch will complain regarding undocumented bindings, so from a > pragmatic point of view the binding must come first. > > Personally, when I read a patch series I do an initial pass in-order, > and having the binding first makes things clearer. I might have some > questions regarding the binding that the driver answers later, and it makes it > easier to spot undocumented properties or conventions used by the > driver. Doing so the other way around usually leaves me with more > questions at the end. > Thanks a lot for the explanation, it certainly makes sense then to have the DT binding before. I'll propose a patch to add that information to Documentation/devicetree/bindings/submitting-patches.txt so people (like me) who didn't find it obvious can know what the convention is. >> That's why what makes the most sense for me is what the old convention did, >> add the DT binding docs in the same patch that implements the binding. > > Having a separate patch for the binding is very helpful for those of us > doing review. For one thing it helps us to find the binding document, > which can be important when a driver is thousands of lines long. For > another it means that we can be clear that our Acked-by, Reviewed-by, > etc apply to the binding and not necessarily the rest of the code. > Agreed. > For small patches, this is obviously less of a concern. > > Thanks, > Mark. > 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 Thu, Oct 09, 2014 at 04:19:47PM +0100, Javier Martinez Canillas wrote: > Hello Mark, > > On 10/09/2014 12:27 PM, Mark Rutland wrote: > >> > >> Well, is not fairly obvious to me. One can also say the opposite, why the > >> kernel is documenting a DT binding that is not (currently) implemented? > > > > Checkpatch will complain regarding undocumented bindings, so from a > > pragmatic point of view the binding must come first. > > > > Personally, when I read a patch series I do an initial pass in-order, > > and having the binding first makes things clearer. I might have some > > questions regarding the binding that the driver answers later, and it makes it > > easier to spot undocumented properties or conventions used by the > > driver. Doing so the other way around usually leaves me with more > > questions at the end. > > > > Thanks a lot for the explanation, it certainly makes sense then to have > the DT binding before. I'll propose a patch to add that information to > Documentation/devicetree/bindings/submitting-patches.txt so people > (like me) who didn't find it obvious can know what the convention is. That sounds like a good idea; yes please. Thanks, Mark. -- 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/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c index 18236be..a076b7f 100644 --- a/drivers/regulator/of_regulator.c +++ b/drivers/regulator/of_regulator.c @@ -93,6 +93,9 @@ static void of_get_regulation_constraints(struct device_node *np, }; } + if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) + constraints->initial_mode = pval; + for (i = 0; i < ARRAY_SIZE(regulator_states); i++) { switch (i) { case PM_SUSPEND_MEM:
The regulator core allows boards to define an initial operating mode to be used as a default for regulators. With board files and platform data, it is possible to fill a struct regulation_constraints .initial_mode to set an initial mode for each regulator. But currently there isn't a way to do the same with DeviceTrees. Argubly the operating modes are Linux-specific so that information should not be in the DT which should be used to only describe hardware. But regulators having different operating modes is also a hardware property since many PMICs have support to set different modes for their regulators. So, the generic REGULATOR_MODE_{FAST,NORMAL,IDLE,STANDBY} modes can be used to describe different level of power efficiency required for each regulator and drivers can map those levels to the real modes supported by the hardware as stated on their data-sheet. Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> --- drivers/regulator/of_regulator.c | 3 +++ 1 file changed, 3 insertions(+)