Message ID | 6ac87dcbb660ae892bf8740c78d3eca7625d6db6.1687814664.git.mirq-linux@rere.qmqm.pl |
---|---|
State | New |
Headers | show |
Series | [1/1] i2c: at91: constify at91_twi_pdata | expand |
Hi Michal, [...] > -static struct at91_twi_pdata *at91_twi_get_driver_data( > +static const struct at91_twi_pdata *at91_twi_get_driver_data( > struct platform_device *pdev) > { > if (pdev->dev.of_node) { > @@ -189,9 +189,9 @@ static struct at91_twi_pdata *at91_twi_get_driver_data( > match = of_match_node(atmel_twi_dt_ids, pdev->dev.of_node); > if (!match) > return NULL; > - return (struct at91_twi_pdata *)match->data; > + return match->data; > } > - return (struct at91_twi_pdata *) platform_get_device_id(pdev)->driver_data; > + return (const void *) platform_get_device_id(pdev)->driver_data; the const's always confuse me... do you get an error here? Is this cast really needed? Andi
On Fri, Jun 30, 2023 at 12:38:25AM +0200, Andi Shyti wrote: > Hi Michal, > > [...] > > > -static struct at91_twi_pdata *at91_twi_get_driver_data( > > +static const struct at91_twi_pdata *at91_twi_get_driver_data( > > struct platform_device *pdev) > > { > > if (pdev->dev.of_node) { > > @@ -189,9 +189,9 @@ static struct at91_twi_pdata *at91_twi_get_driver_data( > > match = of_match_node(atmel_twi_dt_ids, pdev->dev.of_node); > > if (!match) > > return NULL; > > - return (struct at91_twi_pdata *)match->data; > > + return match->data; > > } > > - return (struct at91_twi_pdata *) platform_get_device_id(pdev)->driver_data; > > + return (const void *) platform_get_device_id(pdev)->driver_data; > > the const's always confuse me... do you get an error here? Is > this cast really needed? platform_device_id.driver_data is an ulong, not a void pointer. So, the cast is needed. It could be just (void *), but I think it's better to document the constness in the code.
On 30/06/2023 00:38, Andi Shyti wrote: > Hi Michal, > > [...] > >> -static struct at91_twi_pdata *at91_twi_get_driver_data( >> +static const struct at91_twi_pdata *at91_twi_get_driver_data( >> struct platform_device *pdev) >> { >> if (pdev->dev.of_node) { >> @@ -189,9 +189,9 @@ static struct at91_twi_pdata *at91_twi_get_driver_data( >> match = of_match_node(atmel_twi_dt_ids, pdev->dev.of_node); >> if (!match) >> return NULL; >> - return (struct at91_twi_pdata *)match->data; >> + return match->data; >> } >> - return (struct at91_twi_pdata *) platform_get_device_id(pdev)->driver_data; >> + return (const void *) platform_get_device_id(pdev)->driver_data; > > the const's always confuse me... do you get an error here? Is > this cast really needed? I think this change is not necessary and actually should not matter. See for example drivers/tty/serial/samsung_tty.c after my refactorings in commit 3aec40096550 ("tty: serial: samsung: reduce number of casts"). Best regards, Krzysztof
On Mon, Jul 03, 2023 at 08:23:30PM +0200, Krzysztof Kozlowski wrote: > On 30/06/2023 00:38, Andi Shyti wrote: > > Hi Michal, > >> -static struct at91_twi_pdata *at91_twi_get_driver_data( > >> +static const struct at91_twi_pdata *at91_twi_get_driver_data( > >> struct platform_device *pdev) > >> { > >> if (pdev->dev.of_node) { > >> @@ -189,9 +189,9 @@ static struct at91_twi_pdata *at91_twi_get_driver_data( > >> match = of_match_node(atmel_twi_dt_ids, pdev->dev.of_node); > >> if (!match) > >> return NULL; > >> - return (struct at91_twi_pdata *)match->data; > >> + return match->data; > >> } > >> - return (struct at91_twi_pdata *) platform_get_device_id(pdev)->driver_data; > >> + return (const void *) platform_get_device_id(pdev)->driver_data; > > > > the const's always confuse me... do you get an error here? Is > > this cast really needed? > > I think this change is not necessary and actually should not matter. See > for example drivers/tty/serial/samsung_tty.c after my refactorings in > commit 3aec40096550 ("tty: serial: samsung: reduce number of casts"). I added this two fragments to avoid having suggesting that the pdata might not be in a read-only segment. For the compiler it doesn't matter -- the pointer is cast to const on return. Best Regards Michał Mirosław
diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c index 05ad3bc3578a..4d484e231371 100644 --- a/drivers/i2c/busses/i2c-at91-core.c +++ b/drivers/i2c/busses/i2c-at91-core.c @@ -62,28 +62,28 @@ void at91_init_twi_bus(struct at91_twi_dev *dev) at91_init_twi_bus_master(dev); } -static struct at91_twi_pdata at91rm9200_config = { +static const struct at91_twi_pdata at91rm9200_config = { .clk_max_div = 5, .clk_offset = 3, .has_unre_flag = true, }; -static struct at91_twi_pdata at91sam9261_config = { +static const struct at91_twi_pdata at91sam9261_config = { .clk_max_div = 5, .clk_offset = 4, }; -static struct at91_twi_pdata at91sam9260_config = { +static const struct at91_twi_pdata at91sam9260_config = { .clk_max_div = 7, .clk_offset = 4, }; -static struct at91_twi_pdata at91sam9g20_config = { +static const struct at91_twi_pdata at91sam9g20_config = { .clk_max_div = 7, .clk_offset = 4, }; -static struct at91_twi_pdata at91sam9g10_config = { +static const struct at91_twi_pdata at91sam9g10_config = { .clk_max_div = 7, .clk_offset = 4, }; @@ -110,19 +110,19 @@ static const struct platform_device_id at91_twi_devtypes[] = { }; #if defined(CONFIG_OF) -static struct at91_twi_pdata at91sam9x5_config = { +static const struct at91_twi_pdata at91sam9x5_config = { .clk_max_div = 7, .clk_offset = 4, }; -static struct at91_twi_pdata sama5d4_config = { +static const struct at91_twi_pdata sama5d4_config = { .clk_max_div = 7, .clk_offset = 4, .has_hold_field = true, .has_dig_filtr = true, }; -static struct at91_twi_pdata sama5d2_config = { +static const struct at91_twi_pdata sama5d2_config = { .clk_max_div = 7, .clk_offset = 3, .has_unre_flag = true, @@ -134,7 +134,7 @@ static struct at91_twi_pdata sama5d2_config = { .has_clear_cmd = false, /* due to errata, CLEAR cmd is not working */ }; -static struct at91_twi_pdata sam9x60_config = { +static const struct at91_twi_pdata sam9x60_config = { .clk_max_div = 7, .clk_offset = 3, .has_unre_flag = true, @@ -181,7 +181,7 @@ static const struct of_device_id atmel_twi_dt_ids[] = { MODULE_DEVICE_TABLE(of, atmel_twi_dt_ids); #endif -static struct at91_twi_pdata *at91_twi_get_driver_data( +static const struct at91_twi_pdata *at91_twi_get_driver_data( struct platform_device *pdev) { if (pdev->dev.of_node) { @@ -189,9 +189,9 @@ static struct at91_twi_pdata *at91_twi_get_driver_data( match = of_match_node(atmel_twi_dt_ids, pdev->dev.of_node); if (!match) return NULL; - return (struct at91_twi_pdata *)match->data; + return match->data; } - return (struct at91_twi_pdata *) platform_get_device_id(pdev)->driver_data; + return (const void *) platform_get_device_id(pdev)->driver_data; } static int at91_twi_probe(struct platform_device *pdev) diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c index c0c35785a0dc..0434aabb66fe 100644 --- a/drivers/i2c/busses/i2c-at91-master.c +++ b/drivers/i2c/busses/i2c-at91-master.c @@ -32,7 +32,7 @@ void at91_init_twi_bus_master(struct at91_twi_dev *dev) { - struct at91_twi_pdata *pdata = dev->pdata; + const struct at91_twi_pdata *pdata = dev->pdata; u32 filtr = 0; /* FIFO should be enabled immediately after the software reset */ @@ -67,7 +67,7 @@ void at91_init_twi_bus_master(struct at91_twi_dev *dev) static void at91_calc_twi_clock(struct at91_twi_dev *dev) { int ckdiv, cdiv, div, hold = 0, filter_width = 0; - struct at91_twi_pdata *pdata = dev->pdata; + const struct at91_twi_pdata *pdata = dev->pdata; int offset = pdata->clk_offset; int max_ckdiv = pdata->clk_max_div; struct i2c_timings timings, *t = &timings; diff --git a/drivers/i2c/busses/i2c-at91.h b/drivers/i2c/busses/i2c-at91.h index 942e9c3973bb..f0c80b89e502 100644 --- a/drivers/i2c/busses/i2c-at91.h +++ b/drivers/i2c/busses/i2c-at91.h @@ -147,7 +147,7 @@ struct at91_twi_dev { unsigned transfer_status; struct i2c_adapter adapter; unsigned twi_cwgr_reg; - struct at91_twi_pdata *pdata; + const struct at91_twi_pdata *pdata; bool use_dma; bool use_alt_cmd; bool recv_len_abort;
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> --- drivers/i2c/busses/i2c-at91-core.c | 24 ++++++++++++------------ drivers/i2c/busses/i2c-at91-master.c | 4 ++-- drivers/i2c/busses/i2c-at91.h | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-)