Re: [PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
> The code for the special v1p8 / v2p8 gpios is ugly as sin, it operates on > a global v2p8_gpio value rather then storing info in the gmin_subdev struct, > as such passing the subdev->dev pointer would be simply wrong. AFAICT the > v1p8 / v2p8 gpio code is the only caller passing in a NULL pointer and > as said since thisv1p8 / v2p8 gpio code is only for some special evaluation > boards, silencing the error when these variables are not present actually > is the right thing to do. Unfortunately I don't think it is constrained to RVPs. As with all developer hacks on code that isn't subject to public review at the time they escape into the wild 8( Agreed though. The patch makes sense if you don't want to print anything. > > which if my understanding of the subdevices is correct should pass the > > right valid device field from the atomisp. > > > > Please also cc me if you are proposing patches this driver - and also > > linux-media. > > Sorry about that, I messed up my git send-email foo and send this to > a wrong set of addresses (and also added v5 in the subject which should > not be there) I did send out a fresh-copy with the full 7 patch patch-set > directly after CTRL+c-ing this wrong send-email (which only got the > first 3 patches send). So I discovered just afterwards 8) Alan
Re: [PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
> The code for the special v1p8 / v2p8 gpios is ugly as sin, it operates on > a global v2p8_gpio value rather then storing info in the gmin_subdev struct, > as such passing the subdev->dev pointer would be simply wrong. AFAICT the > v1p8 / v2p8 gpio code is the only caller passing in a NULL pointer and > as said since thisv1p8 / v2p8 gpio code is only for some special evaluation > boards, silencing the error when these variables are not present actually > is the right thing to do. Unfortunately I don't think it is constrained to RVPs. As with all developer hacks on code that isn't subject to public review at the time they escape into the wild 8( Agreed though. The patch makes sense if you don't want to print anything. > > which if my understanding of the subdevices is correct should pass the > > right valid device field from the atomisp. > > > > Please also cc me if you are proposing patches this driver - and also > > linux-media. > > Sorry about that, I messed up my git send-email foo and send this to > a wrong set of addresses (and also added v5 in the subject which should > not be there) I did send out a fresh-copy with the full 7 patch patch-set > directly after CTRL+c-ing this wrong send-email (which only got the > first 3 patches send). So I discovered just afterwards 8) Alan
Re: [PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
Hi, On 28-05-17 19:08, Alan Cox wrote: On Sun, 28 May 2017 14:30:35 +0200 Hans de Goedewrote: Do not call dev_warn with a NULL device, this silence the following 2 warnings: [ 14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO [ 14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO We could switch to using pr_warn for dev == NULL instead, but as comments in the source indicate, the check for these 2 special gmin variables with a NULL device is a workaround for 2 specific evaluation boards, so completely silencing the missing warning for these actually is a good thing. At which point real missing variables won't get reported so NAK. I think the right fix is to make the offending callers pass subdev->dev The code for the special v1p8 / v2p8 gpios is ugly as sin, it operates on a global v2p8_gpio value rather then storing info in the gmin_subdev struct, as such passing the subdev->dev pointer would be simply wrong. AFAICT the v1p8 / v2p8 gpio code is the only caller passing in a NULL pointer and as said since thisv1p8 / v2p8 gpio code is only for some special evaluation boards, silencing the error when these variables are not present actually is the right thing to do. which if my understanding of the subdevices is correct should pass the right valid device field from the atomisp. Please also cc me if you are proposing patches this driver - and also linux-media. Sorry about that, I messed up my git send-email foo and send this to a wrong set of addresses (and also added v5 in the subject which should not be there) I did send out a fresh-copy with the full 7 patch patch-set directly after CTRL+c-ing this wrong send-email (which only got the first 3 patches send). Regards, Hans
Re: [PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
Hi, On 28-05-17 19:08, Alan Cox wrote: On Sun, 28 May 2017 14:30:35 +0200 Hans de Goede wrote: Do not call dev_warn with a NULL device, this silence the following 2 warnings: [ 14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO [ 14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO We could switch to using pr_warn for dev == NULL instead, but as comments in the source indicate, the check for these 2 special gmin variables with a NULL device is a workaround for 2 specific evaluation boards, so completely silencing the missing warning for these actually is a good thing. At which point real missing variables won't get reported so NAK. I think the right fix is to make the offending callers pass subdev->dev The code for the special v1p8 / v2p8 gpios is ugly as sin, it operates on a global v2p8_gpio value rather then storing info in the gmin_subdev struct, as such passing the subdev->dev pointer would be simply wrong. AFAICT the v1p8 / v2p8 gpio code is the only caller passing in a NULL pointer and as said since thisv1p8 / v2p8 gpio code is only for some special evaluation boards, silencing the error when these variables are not present actually is the right thing to do. which if my understanding of the subdevices is correct should pass the right valid device field from the atomisp. Please also cc me if you are proposing patches this driver - and also linux-media. Sorry about that, I messed up my git send-email foo and send this to a wrong set of addresses (and also added v5 in the subject which should not be there) I did send out a fresh-copy with the full 7 patch patch-set directly after CTRL+c-ing this wrong send-email (which only got the first 3 patches send). Regards, Hans
Re: [PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
On Sun, 28 May 2017 14:30:35 +0200 Hans de Goedewrote: > Do not call dev_warn with a NULL device, this silence the following 2 > warnings: > > [ 14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO > [ 14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO > > We could switch to using pr_warn for dev == NULL instead, but as comments > in the source indicate, the check for these 2 special gmin variables with > a NULL device is a workaround for 2 specific evaluation boards, so > completely silencing the missing warning for these actually is a good > thing. At which point real missing variables won't get reported so NAK. I think the right fix is to make the offending callers pass subdev->dev which if my understanding of the subdevices is correct should pass the right valid device field from the atomisp. Please also cc me if you are proposing patches this driver - and also linux-media. Alan
Re: [PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
On Sun, 28 May 2017 14:30:35 +0200 Hans de Goede wrote: > Do not call dev_warn with a NULL device, this silence the following 2 > warnings: > > [ 14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO > [ 14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO > > We could switch to using pr_warn for dev == NULL instead, but as comments > in the source indicate, the check for these 2 special gmin variables with > a NULL device is a workaround for 2 specific evaluation boards, so > completely silencing the missing warning for these actually is a good > thing. At which point real missing variables won't get reported so NAK. I think the right fix is to make the offending callers pass subdev->dev which if my understanding of the subdevices is correct should pass the right valid device field from the atomisp. Please also cc me if you are proposing patches this driver - and also linux-media. Alan
[PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
Do not call dev_warn with a NULL device, this silence the following 2 warnings: [ 14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO [ 14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO We could switch to using pr_warn for dev == NULL instead, but as comments in the source indicate, the check for these 2 special gmin variables with a NULL device is a workaround for 2 specific evaluation boards, so completely silencing the missing warning for these actually is a good thing. Signed-off-by: Hans de Goede--- .../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c| 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c index 104fea2f8697..3fea81ea5dbd 100644 --- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c +++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c @@ -689,7 +689,7 @@ int gmin_get_config_var(struct device *dev, const char *var, char *out, size_t * if (ret == 0) { memcpy(out, ev->var.Data, ev->var.DataSize); *out_len = ev->var.DataSize; - } else { + } else if (dev) { dev_warn(dev, "Failed to find gmin variable %s\n", var8); } -- 2.13.0
[PATCH v5 2/7] staging: atomisp: Do not call dev_warn with a NULL device
Do not call dev_warn with a NULL device, this silence the following 2 warnings: [ 14.392194] (NULL device *): Failed to find gmin variable gmin_V2P8GPIO [ 14.392257] (NULL device *): Failed to find gmin variable gmin_V1P8GPIO We could switch to using pr_warn for dev == NULL instead, but as comments in the source indicate, the check for these 2 special gmin variables with a NULL device is a workaround for 2 specific evaluation boards, so completely silencing the missing warning for these actually is a good thing. Signed-off-by: Hans de Goede --- .../staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c| 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c index 104fea2f8697..3fea81ea5dbd 100644 --- a/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c +++ b/drivers/staging/media/atomisp/platform/intel-mid/atomisp_gmin_platform.c @@ -689,7 +689,7 @@ int gmin_get_config_var(struct device *dev, const char *var, char *out, size_t * if (ret == 0) { memcpy(out, ev->var.Data, ev->var.DataSize); *out_len = ev->var.DataSize; - } else { + } else if (dev) { dev_warn(dev, "Failed to find gmin variable %s\n", var8); } -- 2.13.0