Re: [PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

Hi Mark,

> > The 'quirks' variable cannot ever be negative, therefore use u8
> > instead of int. The 8 bit size is given from the fact that
> > currently the quirks variable has very few statuses.
> > 
> > The rx_lvl_offset and tx_st_done store shift values, so that u8
> > is a proper size.
> > 
> > fifo_lvl_mask stores a series of masks, to be in we will keep the
> > 32 bit size.
> 
> What's the advantage of these changes?  This feels like microptimisation
> of something that shouldn't be a big performance issue and it's not
> always the case that the compiler does something more sensible with
> smaller or unsigned types.  I'm not saying don't do it, I'm just saying
> that it's not clear to me what the win is.

not much indeed. Just something that was a little bothering me
while I was reading through.

Please, feel free to drop it.

Thanks,
Andi

Re: [PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

Hi Mark,

> > The 'quirks' variable cannot ever be negative, therefore use u8
> > instead of int. The 8 bit size is given from the fact that
> > currently the quirks variable has very few statuses.
> > 
> > The rx_lvl_offset and tx_st_done store shift values, so that u8
> > is a proper size.
> > 
> > fifo_lvl_mask stores a series of masks, to be in we will keep the
> > 32 bit size.
> 
> What's the advantage of these changes?  This feels like microptimisation
> of something that shouldn't be a big performance issue and it's not
> always the case that the compiler does something more sensible with
> smaller or unsigned types.  I'm not saying don't do it, I'm just saying
> that it's not clear to me what the win is.

not much indeed. Just something that was a little bothering me
while I was reading through.

Please, feel free to drop it.

Thanks,
Andi

Re: [PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Mark Brown]

On Tue, Jun 28, 2016 at 11:41:15AM +0900, Andi Shyti wrote:
> The 'quirks' variable cannot ever be negative, therefore use u8
> instead of int. The 8 bit size is given from the fact that
> currently the quirks variable has very few statuses.
> 
> The rx_lvl_offset and tx_st_done store shift values, so that u8
> is a proper size.
> 
> fifo_lvl_mask stores a series of masks, to be in we will keep the
> 32 bit size.

What's the advantage of these changes?  This feels like microptimisation
of something that shouldn't be a big performance issue and it's not
always the case that the compiler does something more sensible with
smaller or unsigned types.  I'm not saying don't do it, I'm just saying
that it's not clear to me what the win is.


signature.asc
Description: PGP signature

Re: [PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Mark Brown]

On Tue, Jun 28, 2016 at 11:41:15AM +0900, Andi Shyti wrote:
> The 'quirks' variable cannot ever be negative, therefore use u8
> instead of int. The 8 bit size is given from the fact that
> currently the quirks variable has very few statuses.
> 
> The rx_lvl_offset and tx_st_done store shift values, so that u8
> is a proper size.
> 
> fifo_lvl_mask stores a series of masks, to be in we will keep the
> 32 bit size.

What's the advantage of these changes?  This feels like microptimisation
of something that shouldn't be a big performance issue and it's not
always the case that the compiler does something more sensible with
smaller or unsigned types.  I'm not saying don't do it, I'm just saying
that it's not clear to me what the win is.


signature.asc
Description: PGP signature

[PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

The 'quirks' variable cannot ever be negative, therefore use u8
instead of int. The 8 bit size is given from the fact that
currently the quirks variable has very few statuses.

The rx_lvl_offset and tx_st_done store shift values, so that u8
is a proper size.

fifo_lvl_mask stores a series of masks, to be in we will keep the
32 bit size.

Signed-off-by: Andi Shyti 
Signed-off-by: Jaehoon Chung 
---
 drivers/spi/spi-s3c64xx.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 6d8486f..6c9503a 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -150,10 +150,10 @@ struct s3c64xx_spi_dma_data {
  * which is provided as driver data to the driver.
  */
 struct s3c64xx_spi_port_config {
-   int fifo_lvl_mask[MAX_SPI_PORTS];
-   int rx_lvl_offset;
-   int tx_st_done;
-   int quirks;
+   u32 fifo_lvl_mask[MAX_SPI_PORTS];
+   u8  rx_lvl_offset;
+   u8  tx_st_done;
+   u8  quirks;
boolhigh_speed;
boolclk_from_cmu;
 };
-- 
2.8.1

[PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

The 'quirks' variable cannot ever be negative, therefore use u8
instead of int. The 8 bit size is given from the fact that
currently the quirks variable has very few statuses.

The rx_lvl_offset and tx_st_done store shift values, so that u8
is a proper size.

fifo_lvl_mask stores a series of masks, to be in we will keep the
32 bit size.

Signed-off-by: Andi Shyti 
Signed-off-by: Jaehoon Chung 
---
 drivers/spi/spi-s3c64xx.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 6d8486f..6c9503a 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -150,10 +150,10 @@ struct s3c64xx_spi_dma_data {
  * which is provided as driver data to the driver.
  */
 struct s3c64xx_spi_port_config {
-   int fifo_lvl_mask[MAX_SPI_PORTS];
-   int rx_lvl_offset;
-   int tx_st_done;
-   int quirks;
+   u32 fifo_lvl_mask[MAX_SPI_PORTS];
+   u8  rx_lvl_offset;
+   u8  tx_st_done;
+   u8  quirks;
boolhigh_speed;
boolclk_from_cmu;
 };
-- 
2.8.1