[PATCH V3 05/19] OMAP3+: voltage: use IS_ERR_OR_NULL

David Cohen dacohen at gmail.com
Sat Mar 5 12:36:04 EST 2011


Hi Nishanth,

On Sat, Mar 5, 2011 at 5:29 PM, Nishanth Menon <nm at ti.com> wrote:
> Use IS_ERR_OR_NULL macro instead of !xyz || IS_ERR(xyz) usage.
>
> Signed-off-by: Nishanth Menon <nm at ti.com>
> ---
>  arch/arm/mach-omap2/voltage.c |   22 +++++++++++-----------
>  1 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/voltage.c b/arch/arm/mach-omap2/voltage.c
> index 53c399f..76bcaee 100644
> --- a/arch/arm/mach-omap2/voltage.c
> +++ b/arch/arm/mach-omap2/voltage.c
> @@ -682,7 +682,7 @@ unsigned long omap_voltage_get_nom_volt(struct voltagedomain *voltdm)
>  {
>        struct omap_vdd_info *vdd;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {

I have one comment here. voltdm is received as parameter and it's
already checked by IS_ERR(). Is there any specific reason for that?
IS_ERR() doesn't suppose to be a macro to check if the pointer is
valid or not, but to know if there's an invalid value with error code
in the pointer value. It's useful when you have a function which
returns a pointer but it can return an error code when it fails.
Please, note that IS_ERR() won't detect invalid pointers which does
not represent an error code, so it's not correct to rely on it for
this purpose.
IMO, instead of change to IS_ERR_OR_NULL(), you could only remove
IS_ERR(). The same apply for the other cases.

Kind regards,

David Cohen

>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return 0;
>        }
> @@ -703,7 +703,7 @@ unsigned long omap_vp_get_curr_volt(struct voltagedomain *voltdm)
>        struct omap_vdd_info *vdd;
>        u8 curr_vsel;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return 0;
>        }
> @@ -738,7 +738,7 @@ void omap_vp_enable(struct voltagedomain *voltdm)
>        struct omap_vdd_info *vdd;
>        u32 vpconfig;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return;
>        }
> @@ -776,7 +776,7 @@ void omap_vp_disable(struct voltagedomain *voltdm)
>        u32 vpconfig;
>        int timeout;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return;
>        }
> @@ -829,7 +829,7 @@ int omap_voltage_scale_vdd(struct voltagedomain *voltdm,
>  {
>        struct omap_vdd_info *vdd;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return -EINVAL;
>        }
> @@ -858,7 +858,7 @@ void omap_voltage_reset(struct voltagedomain *voltdm)
>  {
>        unsigned long target_uvdc;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return;
>        }
> @@ -890,7 +890,7 @@ void omap_voltage_get_volttable(struct voltagedomain *voltdm,
>  {
>        struct omap_vdd_info *vdd;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return;
>        }
> @@ -921,7 +921,7 @@ struct omap_volt_data *omap_voltage_get_voltdata(struct voltagedomain *voltdm,
>        struct omap_vdd_info *vdd;
>        int i;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return ERR_PTR(-EINVAL);
>        }
> @@ -959,7 +959,7 @@ int omap_voltage_register_pmic(struct voltagedomain *voltdm,
>  {
>        struct omap_vdd_info *vdd;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return -EINVAL;
>        }
> @@ -986,7 +986,7 @@ struct dentry *omap_voltage_get_dbgdir(struct voltagedomain *voltdm)
>  {
>        struct omap_vdd_info *vdd;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return NULL;
>        }
> @@ -1011,7 +1011,7 @@ void omap_change_voltscale_method(struct voltagedomain *voltdm,
>  {
>        struct omap_vdd_info *vdd;
>
> -       if (!voltdm || IS_ERR(voltdm)) {
> +       if (IS_ERR_OR_NULL(voltdm)) {
>                pr_warning("%s: VDD specified does not exist!\n", __func__);
>                return;
>        }
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



More information about the linux-arm-kernel mailing list