* [PATCH] rtc: remove VLA usage @ 2018-03-07 20:11 Gustavo A. R. Silva 2018-03-07 21:25 ` Alexandre Belloni 0 siblings, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 20:11 UTC (permalink / raw) To: Alessandro Zummo, Alexandre Belloni Cc: linux-rtc, linux-kernel, Gustavo A. R. Silva In preparation to enabling -Wvla, remove VLA and replace it with a fixed-length array instead. Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/rtc/rtc-bq32k.c | 2 +- drivers/rtc/rtc-mcp795.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c index e8698e9..e4b234a 100644 --- a/drivers/rtc/rtc-bq32k.c +++ b/drivers/rtc/rtc-bq32k.c @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) { struct i2c_client *client = to_i2c_client(dev); - uint8_t buffer[len + 1]; + uint8_t buffer[256]; buffer[0] = off; memcpy(&buffer[1], data, len); diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c index 79e24ea..00e11c1 100644 --- a/drivers/rtc/rtc-mcp795.c +++ b/drivers/rtc/rtc-mcp795.c @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) { struct spi_device *spi = to_spi_device(dev); int ret; - u8 tx[2 + count]; + u8 tx[257]; tx[0] = MCP795_WRITE; tx[1] = addr; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 20:11 [PATCH] rtc: remove VLA usage Gustavo A. R. Silva @ 2018-03-07 21:25 ` Alexandre Belloni 2018-03-07 22:39 ` Gustavo A. R. Silva 2018-03-07 22:40 ` Gustavo A. R. Silva 0 siblings, 2 replies; 8+ messages in thread From: Alexandre Belloni @ 2018-03-07 21:25 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Alessandro Zummo, linux-rtc, linux-kernel, Gustavo A. R. Silva On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: > In preparation to enabling -Wvla, remove VLA and replace it > with a fixed-length array instead. > You should probably explain what VLA is and why this is important to do. > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > --- > drivers/rtc/rtc-bq32k.c | 2 +- > drivers/rtc/rtc-mcp795.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c > index e8698e9..e4b234a 100644 > --- a/drivers/rtc/rtc-bq32k.c > +++ b/drivers/rtc/rtc-bq32k.c > @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) > static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) > { > struct i2c_client *client = to_i2c_client(dev); > - uint8_t buffer[len + 1]; > + uint8_t buffer[256]; > You chose to change len to 255, probably because this is a uint8_t but this is way too much for this rtc, it only has 10 consecutive registers. > buffer[0] = off; > memcpy(&buffer[1], data, len); > diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c > index 79e24ea..00e11c1 100644 > --- a/drivers/rtc/rtc-mcp795.c > +++ b/drivers/rtc/rtc-mcp795.c > @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) > { > struct spi_device *spi = to_spi_device(dev); > int ret; > - u8 tx[2 + count]; > + u8 tx[257]; > > tx[0] = MCP795_WRITE; > tx[1] = addr; > -- > 2.7.4 > -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 21:25 ` Alexandre Belloni @ 2018-03-07 22:39 ` Gustavo A. R. Silva 2018-03-07 23:01 ` Alexandre Belloni 2018-03-07 22:40 ` Gustavo A. R. Silva 1 sibling, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 22:39 UTC (permalink / raw) To: Alexandre Belloni, Gustavo A. R. Silva Cc: Alessandro Zummo, linux-rtc, linux-kernel Hi Alexandre, On 03/07/2018 03:25 PM, Alexandre Belloni wrote: > On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wvla, remove VLA and replace it >> with a fixed-length array instead. >> > You should probably explain what VLA is and why this is important to do. Sure. I can elaborate a little bit more. >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> --- >> drivers/rtc/rtc-bq32k.c | 2 +- >> drivers/rtc/rtc-mcp795.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c >> index e8698e9..e4b234a 100644 >> --- a/drivers/rtc/rtc-bq32k.c >> +++ b/drivers/rtc/rtc-bq32k.c >> @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) >> static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) >> { >> struct i2c_client *client = to_i2c_client(dev); >> - uint8_t buffer[len + 1]; >> + uint8_t buffer[256]; >> > You chose to change len to 255, probably because this is a uint8_t but Correct. > this is way too much for this rtc, it only has 10 consecutive registers. In that case probably the best solution is to add the following line to the module: #define MAX_LEN 10 and update the rest of the code as follows: uint8_t buffer[MAX_LEN + 1]; >> buffer[0] = off; >> memcpy(&buffer[1], data, len); >> diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c >> index 79e24ea..00e11c1 100644 >> --- a/drivers/rtc/rtc-mcp795.c >> +++ b/drivers/rtc/rtc-mcp795.c >> @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) >> { >> struct spi_device *spi = to_spi_device(dev); >> int ret; >> - u8 tx[2 + count]; >> + u8 tx[257]; For this particular case it seems to me that the following works just fine: #define MAX_COUNT 7 u8 tx[MAX_COUNT + 2]; What do you think? Thanks for your feedback. -- Gustavo >> >> tx[0] = MCP795_WRITE; >> tx[1] = addr; >> -- >> 2.7.4 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 22:39 ` Gustavo A. R. Silva @ 2018-03-07 23:01 ` Alexandre Belloni 2018-03-07 23:09 ` Gustavo A. R. Silva 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Belloni @ 2018-03-07 23:01 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Gustavo A. R. Silva, Alessandro Zummo, linux-rtc, linux-kernel On 07/03/2018 at 16:39:51 -0600, Gustavo A. R. Silva wrote: > Hi Alexandre, > > On 03/07/2018 03:25 PM, Alexandre Belloni wrote: > > On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: > > > In preparation to enabling -Wvla, remove VLA and replace it > > > with a fixed-length array instead. > > > > > You should probably explain what VLA is and why this is important to do. > > Sure. I can elaborate a little bit more. > > > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > > > --- > > > drivers/rtc/rtc-bq32k.c | 2 +- > > > drivers/rtc/rtc-mcp795.c | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c > > > index e8698e9..e4b234a 100644 > > > --- a/drivers/rtc/rtc-bq32k.c > > > +++ b/drivers/rtc/rtc-bq32k.c > > > @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) > > > static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) > > > { > > > struct i2c_client *client = to_i2c_client(dev); > > > - uint8_t buffer[len + 1]; > > > + uint8_t buffer[256]; > > You chose to change len to 255, probably because this is a uint8_t but > > Correct. > > > this is way too much for this rtc, it only has 10 consecutive registers. > > In that case probably the best solution is to add the following line to the > module: > > #define MAX_LEN 10 > > and update the rest of the code as follows: > > uint8_t buffer[MAX_LEN + 1]; > Seems better, yes. > > > > buffer[0] = off; > > > memcpy(&buffer[1], data, len); > > > diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c > > > index 79e24ea..00e11c1 100644 > > > --- a/drivers/rtc/rtc-mcp795.c > > > +++ b/drivers/rtc/rtc-mcp795.c > > > @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) > > > { > > > struct spi_device *spi = to_spi_device(dev); > > > int ret; > > > - u8 tx[2 + count]; > > > + u8 tx[257]; > > For this particular case it seems to me that the following works just fine: > > #define MAX_COUNT 7 > > u8 tx[MAX_COUNT + 2]; > > What do you think? > Nope, that RTC has a section of 255 bytes that could be read at once so 257 is the correct value. -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 23:01 ` Alexandre Belloni @ 2018-03-07 23:09 ` Gustavo A. R. Silva 2018-03-07 23:12 ` Alexandre Belloni 0 siblings, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 23:09 UTC (permalink / raw) To: Alexandre Belloni Cc: Gustavo A. R. Silva, Alessandro Zummo, linux-rtc, linux-kernel On 03/07/2018 05:01 PM, Alexandre Belloni wrote: > On 07/03/2018 at 16:39:51 -0600, Gustavo A. R. Silva wrote: >> Hi Alexandre, >> >> On 03/07/2018 03:25 PM, Alexandre Belloni wrote: >>> On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: >>>> In preparation to enabling -Wvla, remove VLA and replace it >>>> with a fixed-length array instead. >>>> >>> You should probably explain what VLA is and why this is important to do. >> Sure. I can elaborate a little bit more. >> >>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >>>> --- >>>> drivers/rtc/rtc-bq32k.c | 2 +- >>>> drivers/rtc/rtc-mcp795.c | 2 +- >>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c >>>> index e8698e9..e4b234a 100644 >>>> --- a/drivers/rtc/rtc-bq32k.c >>>> +++ b/drivers/rtc/rtc-bq32k.c >>>> @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) >>>> static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) >>>> { >>>> struct i2c_client *client = to_i2c_client(dev); >>>> - uint8_t buffer[len + 1]; >>>> + uint8_t buffer[256]; >>> You chose to change len to 255, probably because this is a uint8_t but >> Correct. >> >>> this is way too much for this rtc, it only has 10 consecutive registers. >> In that case probably the best solution is to add the following line to the >> module: >> >> #define MAX_LEN 10 >> >> and update the rest of the code as follows: >> >> uint8_t buffer[MAX_LEN + 1]; >> > Seems better, yes. > >>>> buffer[0] = off; >>>> memcpy(&buffer[1], data, len); >>>> diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c >>>> index 79e24ea..00e11c1 100644 >>>> --- a/drivers/rtc/rtc-mcp795.c >>>> +++ b/drivers/rtc/rtc-mcp795.c >>>> @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) >>>> { >>>> struct spi_device *spi = to_spi_device(dev); >>>> int ret; >>>> - u8 tx[2 + count]; >>>> + u8 tx[257]; >> For this particular case it seems to me that the following works just fine: >> >> #define MAX_COUNT 7 >> >> u8 tx[MAX_COUNT + 2]; >> >> What do you think? >> > Nope, that RTC has a section of 255 bytes that could be read at once so > 257 is the correct value. > I see... I was looking into this piece of code drivers/rtc/rtc-mcp795.c:302: tmp[0] = (tmp[0] & 0x80) | bin2bcd(alm->time.tm_sec); tmp[1] = (tmp[1] & 0x80) | bin2bcd(alm->time.tm_min); tmp[2] = (tmp[2] & 0xE0) | bin2bcd(alm->time.tm_hour); tmp[3] = (tmp[3] & 0x80) | bin2bcd(alm->time.tm_wday + 1); /* set alarm match: seconds, minutes, hour, day, date and month */ tmp[3] |= (MCP795_ALM0C2_BIT | MCP795_ALM0C1_BIT | MCP795_ALM0C0_BIT); tmp[4] = (tmp[4] & 0xC0) | bin2bcd(alm->time.tm_mday); tmp[5] = (tmp[5] & 0xE0) | bin2bcd(alm->time.tm_mon + 1); ret = mcp795_rtcc_write(dev, MCP795_REG_ALM0_SECONDS, tmp, sizeof(tmp)); OK. I'll work on v2 of this patch. I'll send it shortly. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 23:09 ` Gustavo A. R. Silva @ 2018-03-07 23:12 ` Alexandre Belloni 2018-03-07 23:14 ` Gustavo A. R. Silva 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Belloni @ 2018-03-07 23:12 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Gustavo A. R. Silva, Alessandro Zummo, linux-rtc, linux-kernel On 07/03/2018 at 17:09:22 -0600, Gustavo A. R. Silva wrote: > > > On 03/07/2018 05:01 PM, Alexandre Belloni wrote: > > On 07/03/2018 at 16:39:51 -0600, Gustavo A. R. Silva wrote: > > > Hi Alexandre, > > > > > > On 03/07/2018 03:25 PM, Alexandre Belloni wrote: > > > > On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: > > > > > In preparation to enabling -Wvla, remove VLA and replace it > > > > > with a fixed-length array instead. > > > > > > > > > You should probably explain what VLA is and why this is important to do. > > > Sure. I can elaborate a little bit more. > > > > > > > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > > > > > --- > > > > > drivers/rtc/rtc-bq32k.c | 2 +- > > > > > drivers/rtc/rtc-mcp795.c | 2 +- > > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c > > > > > index e8698e9..e4b234a 100644 > > > > > --- a/drivers/rtc/rtc-bq32k.c > > > > > +++ b/drivers/rtc/rtc-bq32k.c > > > > > @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) > > > > > static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) > > > > > { > > > > > struct i2c_client *client = to_i2c_client(dev); > > > > > - uint8_t buffer[len + 1]; > > > > > + uint8_t buffer[256]; > > > > You chose to change len to 255, probably because this is a uint8_t but > > > Correct. > > > > > > > this is way too much for this rtc, it only has 10 consecutive registers. > > > In that case probably the best solution is to add the following line to the > > > module: > > > > > > #define MAX_LEN 10 > > > > > > and update the rest of the code as follows: > > > > > > uint8_t buffer[MAX_LEN + 1]; > > > > > Seems better, yes. > > > > > > > buffer[0] = off; > > > > > memcpy(&buffer[1], data, len); > > > > > diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c > > > > > index 79e24ea..00e11c1 100644 > > > > > --- a/drivers/rtc/rtc-mcp795.c > > > > > +++ b/drivers/rtc/rtc-mcp795.c > > > > > @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) > > > > > { > > > > > struct spi_device *spi = to_spi_device(dev); > > > > > int ret; > > > > > - u8 tx[2 + count]; > > > > > + u8 tx[257]; > > > For this particular case it seems to me that the following works just fine: > > > > > > #define MAX_COUNT 7 > > > > > > u8 tx[MAX_COUNT + 2]; > > > > > > What do you think? > > > > > Nope, that RTC has a section of 255 bytes that could be read at once so > > 257 is the correct value. > > > > I see... > > I was looking into this piece of code drivers/rtc/rtc-mcp795.c:302: > > tmp[0] = (tmp[0] & 0x80) | bin2bcd(alm->time.tm_sec); > tmp[1] = (tmp[1] & 0x80) | bin2bcd(alm->time.tm_min); > tmp[2] = (tmp[2] & 0xE0) | bin2bcd(alm->time.tm_hour); > tmp[3] = (tmp[3] & 0x80) | bin2bcd(alm->time.tm_wday + 1); > /* set alarm match: seconds, minutes, hour, day, date and month */ > tmp[3] |= (MCP795_ALM0C2_BIT | MCP795_ALM0C1_BIT | > MCP795_ALM0C0_BIT); > tmp[4] = (tmp[4] & 0xC0) | bin2bcd(alm->time.tm_mday); > tmp[5] = (tmp[5] & 0xE0) | bin2bcd(alm->time.tm_mon + 1); > > ret = mcp795_rtcc_write(dev, MCP795_REG_ALM0_SECONDS, tmp, > sizeof(tmp)); > Yeah, the 128 or 255 bytes NVRAM is not yet supported but this is on my todo list -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 23:12 ` Alexandre Belloni @ 2018-03-07 23:14 ` Gustavo A. R. Silva 0 siblings, 0 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 23:14 UTC (permalink / raw) To: Alexandre Belloni Cc: Gustavo A. R. Silva, Alessandro Zummo, linux-rtc, linux-kernel On 03/07/2018 05:12 PM, Alexandre Belloni wrote: > On 07/03/2018 at 17:09:22 -0600, Gustavo A. R. Silva wrote: >> >> On 03/07/2018 05:01 PM, Alexandre Belloni wrote: >>> On 07/03/2018 at 16:39:51 -0600, Gustavo A. R. Silva wrote: >>>> Hi Alexandre, >>>> >>>> On 03/07/2018 03:25 PM, Alexandre Belloni wrote: >>>>> On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: >>>>>> In preparation to enabling -Wvla, remove VLA and replace it >>>>>> with a fixed-length array instead. >>>>>> >>>>> You should probably explain what VLA is and why this is important to do. >>>> Sure. I can elaborate a little bit more. >>>> >>>>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >>>>>> --- >>>>>> drivers/rtc/rtc-bq32k.c | 2 +- >>>>>> drivers/rtc/rtc-mcp795.c | 2 +- >>>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c >>>>>> index e8698e9..e4b234a 100644 >>>>>> --- a/drivers/rtc/rtc-bq32k.c >>>>>> +++ b/drivers/rtc/rtc-bq32k.c >>>>>> @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) >>>>>> static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) >>>>>> { >>>>>> struct i2c_client *client = to_i2c_client(dev); >>>>>> - uint8_t buffer[len + 1]; >>>>>> + uint8_t buffer[256]; >>>>> You chose to change len to 255, probably because this is a uint8_t but >>>> Correct. >>>> >>>>> this is way too much for this rtc, it only has 10 consecutive registers. >>>> In that case probably the best solution is to add the following line to the >>>> module: >>>> >>>> #define MAX_LEN 10 >>>> >>>> and update the rest of the code as follows: >>>> >>>> uint8_t buffer[MAX_LEN + 1]; >>>> >>> Seems better, yes. >>> >>>>>> buffer[0] = off; >>>>>> memcpy(&buffer[1], data, len); >>>>>> diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c >>>>>> index 79e24ea..00e11c1 100644 >>>>>> --- a/drivers/rtc/rtc-mcp795.c >>>>>> +++ b/drivers/rtc/rtc-mcp795.c >>>>>> @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) >>>>>> { >>>>>> struct spi_device *spi = to_spi_device(dev); >>>>>> int ret; >>>>>> - u8 tx[2 + count]; >>>>>> + u8 tx[257]; >>>> For this particular case it seems to me that the following works just fine: >>>> >>>> #define MAX_COUNT 7 >>>> >>>> u8 tx[MAX_COUNT + 2]; >>>> >>>> What do you think? >>>> >>> Nope, that RTC has a section of 255 bytes that could be read at once so >>> 257 is the correct value. >>> >> I see... >> >> I was looking into this piece of code drivers/rtc/rtc-mcp795.c:302: >> >> tmp[0] = (tmp[0] & 0x80) | bin2bcd(alm->time.tm_sec); >> tmp[1] = (tmp[1] & 0x80) | bin2bcd(alm->time.tm_min); >> tmp[2] = (tmp[2] & 0xE0) | bin2bcd(alm->time.tm_hour); >> tmp[3] = (tmp[3] & 0x80) | bin2bcd(alm->time.tm_wday + 1); >> /* set alarm match: seconds, minutes, hour, day, date and month */ >> tmp[3] |= (MCP795_ALM0C2_BIT | MCP795_ALM0C1_BIT | >> MCP795_ALM0C0_BIT); >> tmp[4] = (tmp[4] & 0xC0) | bin2bcd(alm->time.tm_mday); >> tmp[5] = (tmp[5] & 0xE0) | bin2bcd(alm->time.tm_mon + 1); >> >> ret = mcp795_rtcc_write(dev, MCP795_REG_ALM0_SECONDS, tmp, >> sizeof(tmp)); >> > Yeah, the 128 or 255 bytes NVRAM is not yet supported but this is on my > todo list > I got it. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 21:25 ` Alexandre Belloni 2018-03-07 22:39 ` Gustavo A. R. Silva @ 2018-03-07 22:40 ` Gustavo A. R. Silva 1 sibling, 0 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 22:40 UTC (permalink / raw) To: Alexandre Belloni, Gustavo A. R. Silva Cc: Alessandro Zummo, linux-rtc, linux-kernel Hi Alexandre, On 03/07/2018 03:25 PM, Alexandre Belloni wrote: > On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wvla, remove VLA and replace it >> with a fixed-length array instead. >> > You should probably explain what VLA is and why this is important to do. Sure. I can elaborate a little bit more. >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> --- >> drivers/rtc/rtc-bq32k.c | 2 +- >> drivers/rtc/rtc-mcp795.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c >> index e8698e9..e4b234a 100644 >> --- a/drivers/rtc/rtc-bq32k.c >> +++ b/drivers/rtc/rtc-bq32k.c >> @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) >> static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) >> { >> struct i2c_client *client = to_i2c_client(dev); >> - uint8_t buffer[len + 1]; >> + uint8_t buffer[256]; >> > You chose to change len to 255, probably because this is a uint8_t but Correct. > this is way too much for this rtc, it only has 10 consecutive registers. In that case probably the best solution is to add the following line to the module: #define MAX_LEN 10 and update the rest of the code as follows: uint8_t buffer[MAX_LEN + 1]; >> buffer[0] = off; >> memcpy(&buffer[1], data, len); >> diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c >> index 79e24ea..00e11c1 100644 >> --- a/drivers/rtc/rtc-mcp795.c >> +++ b/drivers/rtc/rtc-mcp795.c >> @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) >> { >> struct spi_device *spi = to_spi_device(dev); >> int ret; >> - u8 tx[2 + count]; >> + u8 tx[257]; For this particular case it seems to me that the following works just fine: #define MAX_COUNT 7 u8 tx[MAX_COUNT + 2]; What do you think? Thanks for your feedback. -- Gustavo >> >> tx[0] = MCP795_WRITE; >> tx[1] = addr; >> -- >> 2.7.4 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-07 23:34 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-07 20:11 [PATCH] rtc: remove VLA usage Gustavo A. R. Silva 2018-03-07 21:25 ` Alexandre Belloni 2018-03-07 22:39 ` Gustavo A. R. Silva 2018-03-07 23:01 ` Alexandre Belloni 2018-03-07 23:09 ` Gustavo A. R. Silva 2018-03-07 23:12 ` Alexandre Belloni 2018-03-07 23:14 ` Gustavo A. R. Silva 2018-03-07 22:40 ` Gustavo A. R. Silva
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.