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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Nishanth,

On Sat, Mar 5, 2011 at 5:29 PM, Nishanth Menon <nm@xxxxxx> wrote:
> Use IS_ERR_OR_NULL macro instead of !xyz || IS_ERR(xyz) usage.
>
> Signed-off-by: Nishanth Menon <nm@xxxxxx>
> ---
> Â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@xxxxxxxxxxxxxxx
> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux