| Message ID | 1433460203-7043-1-git-send-email-angelo@sysam.it |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
On 5 June 2015 at 04:53, Angelo Dureghello <angelo@sysam.it> wrote: > Fix clamp macro redefined warning, and clamp type check warnings. > > Signed-off-by: Angelo Dureghello <angelo@sysam.it> > --- > drivers/spi/cf_qspi.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/spi/cf_qspi.c b/drivers/spi/cf_qspi.c > index 834c5bd..c4bafe0 100644 > --- a/drivers/spi/cf_qspi.c > +++ b/drivers/spi/cf_qspi.c > @@ -19,7 +19,6 @@ > > DECLARE_GLOBAL_DATA_PTR; > > -#define clamp(x, low, high) (min(max(low, x), high)) > #define to_cf_qspi_slave(s) container_of(s, struct cf_qspi_slave, slave) > > struct cf_qspi_slave { > @@ -119,7 +118,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, > if (max_hz == 0) /* Go as fast as possible */ > dev->qmr = 2u; > else /* Get the closest baud rate */ > - dev->qmr = clamp(((gd->bus_clk >> 2) + max_hz - 1)/max_hz, > + dev->qmr = clamp_val(((gd->bus_clk >> 2) + max_hz - 1)/max_hz, Yes, there is a common macro in include/linux - can you check this clamp or clamp_val and test it accordingly. > 2u, 255u); > > /* Map mode to QMR[CPOL] and QMR[CPHA] */ > -- > 2.1.4 > thanks!
On 19/06/2015 09:43, Jagan Teki wrote: > On 5 June 2015 at 04:53, Angelo Dureghello <angelo@sysam.it> wrote: >> Fix clamp macro redefined warning, and clamp type check warnings. >> >> Signed-off-by: Angelo Dureghello <angelo@sysam.it> >> --- >> drivers/spi/cf_qspi.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/spi/cf_qspi.c b/drivers/spi/cf_qspi.c >> index 834c5bd..c4bafe0 100644 >> --- a/drivers/spi/cf_qspi.c >> +++ b/drivers/spi/cf_qspi.c >> @@ -19,7 +19,6 @@ >> >> DECLARE_GLOBAL_DATA_PTR; >> >> -#define clamp(x, low, high) (min(max(low, x), high)) >> #define to_cf_qspi_slave(s) container_of(s, struct cf_qspi_slave, slave) >> >> struct cf_qspi_slave { >> @@ -119,7 +118,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, >> if (max_hz == 0) /* Go as fast as possible */ >> dev->qmr = 2u; >> else /* Get the closest baud rate */ >> - dev->qmr = clamp(((gd->bus_clk >> 2) + max_hz - 1)/max_hz, >> + dev->qmr = clamp_val(((gd->bus_clk >> 2) + max_hz - 1)/max_hz, > > Yes, there is a common macro in include/linux - can you check this > clamp or clamp_val and > test it accordingly. > Hi Jagan, i realized the warnings was correctly generated from the "unnecessary" pointer comparison used from the clamp/min/max macros. More correct solution here is to set 2u, 255u to 2lu, 255lu, and types are compliant to the comparison, so warnings are not thrown anymore. The clamp macro defined in this file seems anyway still not needed (redefine warning) since it is exactly the same in the commonly used include/linux/kernel.h clamp macro. So i post a v2 version of the patch with these 2 changes. Ok ? Best regards Angelo Dureghello
On 21 June 2015 at 14:39, Angelo Dureghello <angelo@sysam.it> wrote: > > > On 19/06/2015 09:43, Jagan Teki wrote: >> >> On 5 June 2015 at 04:53, Angelo Dureghello <angelo@sysam.it> wrote: >>> >>> Fix clamp macro redefined warning, and clamp type check warnings. >>> >>> Signed-off-by: Angelo Dureghello <angelo@sysam.it> >>> --- >>> drivers/spi/cf_qspi.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/drivers/spi/cf_qspi.c b/drivers/spi/cf_qspi.c >>> index 834c5bd..c4bafe0 100644 >>> --- a/drivers/spi/cf_qspi.c >>> +++ b/drivers/spi/cf_qspi.c >>> @@ -19,7 +19,6 @@ >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> -#define clamp(x, low, high) (min(max(low, x), high)) >>> #define to_cf_qspi_slave(s) container_of(s, struct cf_qspi_slave, >>> slave) >>> >>> struct cf_qspi_slave { >>> @@ -119,7 +118,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, >>> unsigned int cs, >>> if (max_hz == 0) /* Go as fast as possible */ >>> dev->qmr = 2u; >>> else /* Get the closest baud rate */ >>> - dev->qmr = clamp(((gd->bus_clk >> 2) + max_hz - >>> 1)/max_hz, >>> + dev->qmr = clamp_val(((gd->bus_clk >> 2) + max_hz - >>> 1)/max_hz, >> >> >> Yes, there is a common macro in include/linux - can you check this >> clamp or clamp_val and >> test it accordingly. >> > > Hi Jagan, > > i realized the warnings was correctly generated from the "unnecessary" > pointer comparison used from the clamp/min/max macros. > > More correct solution here is to set 2u, 255u to 2lu, 255lu, and > types are compliant to the comparison, so warnings are not thrown anymore. > > The clamp macro defined in this file seems anyway still not needed (redefine > warning) since it is exactly the same in the commonly used > include/linux/kernel.h clamp macro. > > So i post a v2 version of the patch with these 2 changes. Ok ? Yes, please. thanks!
diff --git a/drivers/spi/cf_qspi.c b/drivers/spi/cf_qspi.c index 834c5bd..c4bafe0 100644 --- a/drivers/spi/cf_qspi.c +++ b/drivers/spi/cf_qspi.c @@ -19,7 +19,6 @@ DECLARE_GLOBAL_DATA_PTR; -#define clamp(x, low, high) (min(max(low, x), high)) #define to_cf_qspi_slave(s) container_of(s, struct cf_qspi_slave, slave) struct cf_qspi_slave { @@ -119,7 +118,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, if (max_hz == 0) /* Go as fast as possible */ dev->qmr = 2u; else /* Get the closest baud rate */ - dev->qmr = clamp(((gd->bus_clk >> 2) + max_hz - 1)/max_hz, + dev->qmr = clamp_val(((gd->bus_clk >> 2) + max_hz - 1)/max_hz, 2u, 255u); /* Map mode to QMR[CPOL] and QMR[CPHA] */
Fix clamp macro redefined warning, and clamp type check warnings. Signed-off-by: Angelo Dureghello <angelo@sysam.it> --- drivers/spi/cf_qspi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)