diff mbox series

media: ccs: Print a warning on CCS static data parse failure

Message ID 20241209111738.79679-1-mehdi.djait@linux.intel.com (mailing list archive)
State New
Headers show
Series media: ccs: Print a warning on CCS static data parse failure | expand

Commit Message

Mehdi Djait Dec. 9, 2024, 11:17 a.m. UTC
ccs_data_parse() return value is not propagated up to the probe
function, making it difficult on static data parse Failure.
Improve this by printing a warning when ccs_data_parse() fails.

Signed-off-by: Mehdi Djait <mehdi.djait@linux.intel.com>
---
 drivers/media/i2c/ccs/ccs-data.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Sakari Ailus Dec. 9, 2024, 11:40 a.m. UTC | #1
Hi Mehdi,

On Mon, Dec 09, 2024 at 12:17:38PM +0100, Mehdi Djait wrote:
> ccs_data_parse() return value is not propagated up to the probe
> function, making it difficult on static data parse Failure.

Could you reword this by referring to the CCS static data library only?
This would fit better for the CCS tools where the patch will also be
imported to.

<URL:https://github.com/MIPI-Alliance/ccs-tools>

I also wonder if the CCS driver should actually fail probe if parsing
fails: this is a very likely problem and quitting there would be
appropriate IMO. Not every device might need one, so it should be just
based on parser failure.

Thanks.

> Improve this by printing a warning when ccs_data_parse() fails.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait@linux.intel.com>
> ---
>  drivers/media/i2c/ccs/ccs-data.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/i2c/ccs/ccs-data.c b/drivers/media/i2c/ccs/ccs-data.c
> index c40d859166dd..f64fbfa1c7b5 100644
> --- a/drivers/media/i2c/ccs/ccs-data.c
> +++ b/drivers/media/i2c/ccs/ccs-data.c
> @@ -976,6 +976,7 @@ int ccs_data_parse(struct ccs_data_container *ccsdata, const void *data,
>  out_cleanup:
>  	kvfree(bin.base);
>  	memset(ccsdata, 0, sizeof(*ccsdata));
> +	dev_warn(dev, "failed to parse CCS static data file: %d\n", rval);

s/ file//

>  
>  	return rval;
>  }
Mehdi Djait Dec. 10, 2024, 8:53 a.m. UTC | #2
Hi Sakari,

On Mon, Dec 09, 2024 at 11:40:00AM +0000, Sakari Ailus wrote:
> Hi Mehdi,
> 
> On Mon, Dec 09, 2024 at 12:17:38PM +0100, Mehdi Djait wrote:
> > ccs_data_parse() return value is not propagated up to the probe
> > function, making it difficult on static data parse Failure.
> 
> Could you reword this by referring to the CCS static data library only?
> This would fit better for the CCS tools where the patch will also be
> imported to.
> 
> <URL:https://github.com/MIPI-Alliance/ccs-tools>
> 
> I also wonder if the CCS driver should actually fail probe if parsing
> fails: this is a very likely problem and quitting there would be
> appropriate IMO. Not every device might need one, so it should be just
> based on parser failure.

I would also think there is no point to continue probing if parsing fails. I
will send a v2 for this.

--
Kind Regards
Mehdi Djait
diff mbox series

Patch

diff --git a/drivers/media/i2c/ccs/ccs-data.c b/drivers/media/i2c/ccs/ccs-data.c
index c40d859166dd..f64fbfa1c7b5 100644
--- a/drivers/media/i2c/ccs/ccs-data.c
+++ b/drivers/media/i2c/ccs/ccs-data.c
@@ -976,6 +976,7 @@  int ccs_data_parse(struct ccs_data_container *ccsdata, const void *data,
 out_cleanup:
 	kvfree(bin.base);
 	memset(ccsdata, 0, sizeof(*ccsdata));
+	dev_warn(dev, "failed to parse CCS static data file: %d\n", rval);
 
 	return rval;
 }