| Message ID | E1aAO8O-0007xP-Do@rmk-PC.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19 December 2015 at 21:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > Each time a driver such as sdhci-esdhc-imx is probed, we get a info > printk complaining that the DT voltage-ranges property has not been > specified. > > However, the DT binding specifically says that the voltage-ranges > property is optional. That means we should not be complaining that > DT hasn't specified this property: by indicating that it's optional, > it is valid not to have the property in DT. Agree! > > Silence the warning if the property is missing. > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > --- > drivers/mmc/core/core.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 5ae89e48fd85..b5e663b67cb5 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -1220,8 +1220,12 @@ int mmc_of_parse_voltage(struct device_node *np, u32 *mask) > > voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges); > num_ranges = num_ranges / sizeof(*voltage_ranges) / 2; > - if (!voltage_ranges || !num_ranges) { > - pr_info("%s: voltage-ranges unspecified\n", np->full_name); > + if (!voltage_ranges) { > + pr_debug("%s: voltage-ranges unspecified\n", np->full_name); > + return -EINVAL; Because it's optional, I don't think we should return an error code here. > + } > + if (!num_ranges) { > + pr_err("%s: voltage-ranges empty\n", np->full_name); > return -EINVAL; Ditto. > } > > -- > 2.1.0 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 21, 2015 at 11:18:15AM +0100, Ulf Hansson wrote: > On 19 December 2015 at 21:29, Russell King <rmk+kernel@arm.linux.org.uk> wrote: > > Each time a driver such as sdhci-esdhc-imx is probed, we get a info > > printk complaining that the DT voltage-ranges property has not been > > specified. > > > > However, the DT binding specifically says that the voltage-ranges > > property is optional. That means we should not be complaining that > > DT hasn't specified this property: by indicating that it's optional, > > it is valid not to have the property in DT. > > Agree! > > > > > Silence the warning if the property is missing. > > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> > > --- > > drivers/mmc/core/core.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > > index 5ae89e48fd85..b5e663b67cb5 100644 > > --- a/drivers/mmc/core/core.c > > +++ b/drivers/mmc/core/core.c > > @@ -1220,8 +1220,12 @@ int mmc_of_parse_voltage(struct device_node *np, u32 *mask) > > > > voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges); > > num_ranges = num_ranges / sizeof(*voltage_ranges) / 2; > > - if (!voltage_ranges || !num_ranges) { > > - pr_info("%s: voltage-ranges unspecified\n", np->full_name); > > + if (!voltage_ranges) { > > + pr_debug("%s: voltage-ranges unspecified\n", np->full_name); > > + return -EINVAL; > > Because it's optional, I don't think we should return an error code here. Maybe, but that's changing the behaviour beyond what I said in the commit message, hence should be a separate patch so that if it causes problems, then it can be reverted independently of this fix. > > + } > > + if (!num_ranges) { > > + pr_err("%s: voltage-ranges empty\n", np->full_name); > > return -EINVAL; > > Ditto. For this one, it's entirely reasonable - it's a mal-formed voltage-ranges property.
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index 5ae89e48fd85..b5e663b67cb5 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -1220,8 +1220,12 @@ int mmc_of_parse_voltage(struct device_node *np, u32 *mask) voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges); num_ranges = num_ranges / sizeof(*voltage_ranges) / 2; - if (!voltage_ranges || !num_ranges) { - pr_info("%s: voltage-ranges unspecified\n", np->full_name); + if (!voltage_ranges) { + pr_debug("%s: voltage-ranges unspecified\n", np->full_name); + return -EINVAL; + } + if (!num_ranges) { + pr_err("%s: voltage-ranges empty\n", np->full_name); return -EINVAL; }
Each time a driver such as sdhci-esdhc-imx is probed, we get a info printk complaining that the DT voltage-ranges property has not been specified. However, the DT binding specifically says that the voltage-ranges property is optional. That means we should not be complaining that DT hasn't specified this property: by indicating that it's optional, it is valid not to have the property in DT. Silence the warning if the property is missing. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- drivers/mmc/core/core.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)