Re: [U-Boot] [PATCH v4 21/24] spi: mxs_spi: Use GENMASK

2015-10-23 Thread Jagan Teki 
On 23 October 2015 at 02:48, Fabio Estevam  wrote:
> On Thu, Oct 22, 2015 at 7:15 PM, Jagan Teki  wrote:
>
  #defineMXS_SPI_MAX_TIMEOUT 100
  #defineMXS_SPI_PORT_OFFSET 0x2000
 -#define MXS_SSP_CHIPSELECT_MASK0x0030
 +#define MXS_SSP_CHIPSELECT_MASKGENMASK(21, 20)
>>>
>>> Does this really improve the code?
>>
>> GENMASK will simplificate the bit masking and most of drivers in Linux
>> start using this along with BIT for bit shifting.
>
> Most drivers?

Most drivers will include all drivers in Linux not only spi drivers.

>
> In mainline kernel:
>
> git grep GENMASK drivers/spi/ | wc -l
> 2

# grep -R GENMASK drivers/ | wc -l
465

>
> No, this is not most drivers ;-)

-- 
Jagan | openedev.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 21/24] spi: mxs_spi: Use GENMASK

2015-10-23 Thread Jagan Teki 
On 23 October 2015 at 03:00, Marek Vasut  wrote:
> On Thursday, October 22, 2015 at 11:15:26 PM, Jagan Teki wrote:
>> On 23 October 2015 at 02:38, Fabio Estevam  wrote:
>> > On Thu, Oct 22, 2015 at 6:50 PM, Jagan Teki  wrote:
>> >> Replace numeric mask hexcodes with GENMASK macro in mxs_spi
>> >>
>> >> Cc: Marek Vasut 
>> >> Signed-off-by: Jagan Teki 
>> >> ---
>> >>
>> >>  drivers/spi/mxs_spi.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
>> >> index 627644b..459c603 100644
>> >> --- a/drivers/spi/mxs_spi.c
>> >> +++ b/drivers/spi/mxs_spi.c
>> >> @@ -23,7 +23,7 @@
>> >>
>> >>  #defineMXS_SPI_MAX_TIMEOUT 100
>> >>  #defineMXS_SPI_PORT_OFFSET 0x2000
>> >>
>> >> -#define MXS_SSP_CHIPSELECT_MASK0x0030
>> >> +#define MXS_SSP_CHIPSELECT_MASKGENMASK(21, 20)
>> >
>> > Does this really improve the code?
>>
>> GENMASK will simplificate
>
> You mean [1] ? :-) In that case, I agree ...
> "
> simplificate

I just used simplificate as per the sentence formation which I would
it's reasonable. Look like simplificate similar to simplify [1]

[1] http://www.dictionarist.com/english-english/simplificate

>
> verb - the action of taking an answered that is simplified and extending the
> answer to the point of complete and utter obscurity, unreadable by anyone, yet
> still correct.
>
> See also: BS
> "
>
> [1] http://cs.urbandictionary.com/define.php?term=simplificate=2897356
>
>> the bit masking and most of drivers in Linux
>> start using this along with BIT for bit shifting.
>>
>> > Personally I prefer the original code as I don't need to go and look
>> > at the definition of the GENMASK() macro.

-- 
Jagan | openedev.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v4 21/24] spi: mxs_spi: Use GENMASK

2015-10-22 Thread Jagan Teki 
Replace numeric mask hexcodes with GENMASK macro in mxs_spi

Cc: Marek Vasut 
Signed-off-by: Jagan Teki 
---
 drivers/spi/mxs_spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
index 627644b..459c603 100644
--- a/drivers/spi/mxs_spi.c
+++ b/drivers/spi/mxs_spi.c
@@ -23,7 +23,7 @@
 
 #defineMXS_SPI_MAX_TIMEOUT 100
 #defineMXS_SPI_PORT_OFFSET 0x2000
-#define MXS_SSP_CHIPSELECT_MASK0x0030
+#define MXS_SSP_CHIPSELECT_MASKGENMASK(21, 20)
 #define MXS_SSP_CHIPSELECT_SHIFT   20
 
 #define MXSSSP_SMALL_TRANSFER  512
-- 
1.9.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 21/24] spi: mxs_spi: Use GENMASK

2015-10-22 Thread Tom Rini 
On Thu, Oct 22, 2015 at 07:08:32PM -0200, Fabio Estevam wrote:
> On Thu, Oct 22, 2015 at 6:50 PM, Jagan Teki  wrote:
> > Replace numeric mask hexcodes with GENMASK macro in mxs_spi
> >
> > Cc: Marek Vasut 
> > Signed-off-by: Jagan Teki 
> > ---
> >  drivers/spi/mxs_spi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> > index 627644b..459c603 100644
> > --- a/drivers/spi/mxs_spi.c
> > +++ b/drivers/spi/mxs_spi.c
> > @@ -23,7 +23,7 @@
> >
> >  #defineMXS_SPI_MAX_TIMEOUT 100
> >  #defineMXS_SPI_PORT_OFFSET 0x2000
> > -#define MXS_SSP_CHIPSELECT_MASK0x0030
> > +#define MXS_SSP_CHIPSELECT_MASKGENMASK(21, 20)
> 
> Does this really improve the code?
> 
> Personally I prefer the original code as I don't need to go and look
> at the definition of the GENMASK() macro.

Fair point.  This is a kernel helper macro but it's "new" and still
gaining traction.  I personally do find GENMASK(hi, lo) and BIT(x) more
readable.  Others don't.  That's fine.  The question I have here is, who
is spending the most time in these drivers?

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 21/24] spi: mxs_spi: Use GENMASK

2015-10-22 Thread Jagan Teki 
On 23 October 2015 at 02:38, Fabio Estevam  wrote:
> On Thu, Oct 22, 2015 at 6:50 PM, Jagan Teki  wrote:
>> Replace numeric mask hexcodes with GENMASK macro in mxs_spi
>>
>> Cc: Marek Vasut 
>> Signed-off-by: Jagan Teki 
>> ---
>>  drivers/spi/mxs_spi.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
>> index 627644b..459c603 100644
>> --- a/drivers/spi/mxs_spi.c
>> +++ b/drivers/spi/mxs_spi.c
>> @@ -23,7 +23,7 @@
>>
>>  #defineMXS_SPI_MAX_TIMEOUT 100
>>  #defineMXS_SPI_PORT_OFFSET 0x2000
>> -#define MXS_SSP_CHIPSELECT_MASK0x0030
>> +#define MXS_SSP_CHIPSELECT_MASKGENMASK(21, 20)
>
> Does this really improve the code?

GENMASK will simplificate the bit masking and most of drivers in Linux
start using this along with BIT for bit shifting.

>
> Personally I prefer the original code as I don't need to go and look
> at the definition of the GENMASK() macro.

-- 
Jagan | openedev.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 21/24] spi: mxs_spi: Use GENMASK

2015-10-22 Thread Fabio Estevam 
On Thu, Oct 22, 2015 at 7:15 PM, Jagan Teki  wrote:

>>>  #defineMXS_SPI_MAX_TIMEOUT 100
>>>  #defineMXS_SPI_PORT_OFFSET 0x2000
>>> -#define MXS_SSP_CHIPSELECT_MASK0x0030
>>> +#define MXS_SSP_CHIPSELECT_MASKGENMASK(21, 20)
>>
>> Does this really improve the code?
>
> GENMASK will simplificate the bit masking and most of drivers in Linux
> start using this along with BIT for bit shifting.

Most drivers?

In mainline kernel:

git grep GENMASK drivers/spi/ | wc -l
2

No, this is not most drivers ;-)
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 21/24] spi: mxs_spi: Use GENMASK

2015-10-22 Thread Marek Vasut 
On Thursday, October 22, 2015 at 11:15:26 PM, Jagan Teki wrote:
> On 23 October 2015 at 02:38, Fabio Estevam  wrote:
> > On Thu, Oct 22, 2015 at 6:50 PM, Jagan Teki  wrote:
> >> Replace numeric mask hexcodes with GENMASK macro in mxs_spi
> >> 
> >> Cc: Marek Vasut 
> >> Signed-off-by: Jagan Teki 
> >> ---
> >> 
> >>  drivers/spi/mxs_spi.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> >> index 627644b..459c603 100644
> >> --- a/drivers/spi/mxs_spi.c
> >> +++ b/drivers/spi/mxs_spi.c
> >> @@ -23,7 +23,7 @@
> >> 
> >>  #defineMXS_SPI_MAX_TIMEOUT 100
> >>  #defineMXS_SPI_PORT_OFFSET 0x2000
> >> 
> >> -#define MXS_SSP_CHIPSELECT_MASK0x0030
> >> +#define MXS_SSP_CHIPSELECT_MASKGENMASK(21, 20)
> > 
> > Does this really improve the code?
> 
> GENMASK will simplificate

You mean [1] ? :-) In that case, I agree ...
"
simplificate

verb - the action of taking an answered that is simplified and extending the 
answer to the point of complete and utter obscurity, unreadable by anyone, yet 
still correct.

See also: BS 
"

[1] http://cs.urbandictionary.com/define.php?term=simplificate=2897356

> the bit masking and most of drivers in Linux
> start using this along with BIT for bit shifting.
> 
> > Personally I prefer the original code as I don't need to go and look
> > at the definition of the GENMASK() macro.

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v4 21/24] spi: mxs_spi: Use GENMASK

2015-10-22 Thread Fabio Estevam 
On Thu, Oct 22, 2015 at 6:50 PM, Jagan Teki  wrote:
> Replace numeric mask hexcodes with GENMASK macro in mxs_spi
>
> Cc: Marek Vasut 
> Signed-off-by: Jagan Teki 
> ---
>  drivers/spi/mxs_spi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/mxs_spi.c b/drivers/spi/mxs_spi.c
> index 627644b..459c603 100644
> --- a/drivers/spi/mxs_spi.c
> +++ b/drivers/spi/mxs_spi.c
> @@ -23,7 +23,7 @@
>
>  #defineMXS_SPI_MAX_TIMEOUT 100
>  #defineMXS_SPI_PORT_OFFSET 0x2000
> -#define MXS_SSP_CHIPSELECT_MASK0x0030
> +#define MXS_SSP_CHIPSELECT_MASKGENMASK(21, 20)

Does this really improve the code?

Personally I prefer the original code as I don't need to go and look
at the definition of the GENMASK() macro.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot