| Message ID | 1445657950-7117-21-git-send-email-jteki@openedev.com |
|---|---|
| State | Rejected |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
On Saturday, October 24, 2015 at 05:39:07 AM, Jagan Teki wrote: > Replace numeric mask hexcodes with GENMASK macro > in mxs_spi > > Cc: Marek Vasut <marex@denx.de> > Signed-off-by: Jagan Teki <jteki@openedev.com> > --- > 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 @@ > > #define MXS_SPI_MAX_TIMEOUT 1000000 > #define MXS_SPI_PORT_OFFSET 0x2000 > -#define MXS_SSP_CHIPSELECT_MASK 0x00300000 > +#define MXS_SSP_CHIPSELECT_MASK GENMASK(21, 20) > #define MXS_SSP_CHIPSELECT_SHIFT 20 This is just making things unreadable, please keep it as is. NAK. Best regards, Marek Vasut
On 24 October 2015 at 18:10, Marek Vasut <marex@denx.de> wrote: > On Saturday, October 24, 2015 at 05:39:07 AM, Jagan Teki wrote: >> Replace numeric mask hexcodes with GENMASK macro >> in mxs_spi >> >> Cc: Marek Vasut <marex@denx.de> >> Signed-off-by: Jagan Teki <jteki@openedev.com> >> --- >> 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 @@ >> >> #define MXS_SPI_MAX_TIMEOUT 1000000 >> #define MXS_SPI_PORT_OFFSET 0x2000 >> -#define MXS_SSP_CHIPSELECT_MASK 0x00300000 >> +#define MXS_SSP_CHIPSELECT_MASK GENMASK(21, 20) >> #define MXS_SSP_CHIPSELECT_SHIFT 20 > > This is just making things unreadable, please keep it as is. NAK. What's wrong with the GENMASK here is that something that you against with it? It don't look like unreadable.
On Saturday, October 24, 2015 at 03:42:43 PM, Jagan Teki wrote: > On 24 October 2015 at 18:10, Marek Vasut <marex@denx.de> wrote: > > On Saturday, October 24, 2015 at 05:39:07 AM, Jagan Teki wrote: > >> Replace numeric mask hexcodes with GENMASK macro > >> in mxs_spi > >> > >> Cc: Marek Vasut <marex@denx.de> > >> Signed-off-by: Jagan Teki <jteki@openedev.com> > >> --- > >> > >> 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 @@ > >> > >> #define MXS_SPI_MAX_TIMEOUT 1000000 > >> #define MXS_SPI_PORT_OFFSET 0x2000 > >> > >> -#define MXS_SSP_CHIPSELECT_MASK 0x00300000 > >> +#define MXS_SSP_CHIPSELECT_MASK GENMASK(21, 20) > >> > >> #define MXS_SSP_CHIPSELECT_SHIFT 20 > > > > This is just making things unreadable, please keep it as is. NAK. > > What's wrong with the GENMASK here is that something that you against > with it? It don't look like unreadable. If I open the datasheet, I can easily locate mask 0x0030_0000 and figure out which bits I need to work with. With genmask ... not so much. It only obfuscates the code. Best regards, Marek Vasut
On Sat, Oct 24, 2015 at 03:48:14PM +0200, Marek Vasut wrote: > On Saturday, October 24, 2015 at 03:42:43 PM, Jagan Teki wrote: > > On 24 October 2015 at 18:10, Marek Vasut <marex@denx.de> wrote: > > > On Saturday, October 24, 2015 at 05:39:07 AM, Jagan Teki wrote: > > >> Replace numeric mask hexcodes with GENMASK macro > > >> in mxs_spi > > >> > > >> Cc: Marek Vasut <marex@denx.de> > > >> Signed-off-by: Jagan Teki <jteki@openedev.com> > > >> --- > > >> > > >> 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 @@ > > >> > > >> #define MXS_SPI_MAX_TIMEOUT 1000000 > > >> #define MXS_SPI_PORT_OFFSET 0x2000 > > >> > > >> -#define MXS_SSP_CHIPSELECT_MASK 0x00300000 > > >> +#define MXS_SSP_CHIPSELECT_MASK GENMASK(21, 20) > > >> > > >> #define MXS_SSP_CHIPSELECT_SHIFT 20 > > > > > > This is just making things unreadable, please keep it as is. NAK. > > > > What's wrong with the GENMASK here is that something that you against > > with it? It don't look like unreadable. > > If I open the datasheet, I can easily locate mask 0x0030_0000 and figure > out which bits I need to work with. With genmask ... not so much. It only > obfuscates the code. Really? I don't have the "mxs" datasheet handy but I have the mx6 solo/duallite one handy and the SPI chapter talks about bits and has them broken down that way, not the hex numbers for masking whatever field. This matched my expectation on how I recall the TI parts being as well, bit field descriptions and binary values, not hex.
On Saturday, October 24, 2015 at 11:49:43 PM, Tom Rini wrote: > On Sat, Oct 24, 2015 at 03:48:14PM +0200, Marek Vasut wrote: > > On Saturday, October 24, 2015 at 03:42:43 PM, Jagan Teki wrote: > > > On 24 October 2015 at 18:10, Marek Vasut <marex@denx.de> wrote: > > > > On Saturday, October 24, 2015 at 05:39:07 AM, Jagan Teki wrote: > > > >> Replace numeric mask hexcodes with GENMASK macro > > > >> in mxs_spi > > > >> > > > >> Cc: Marek Vasut <marex@denx.de> > > > >> Signed-off-by: Jagan Teki <jteki@openedev.com> > > > >> --- > > > >> > > > >> 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 @@ > > > >> > > > >> #define MXS_SPI_MAX_TIMEOUT 1000000 > > > >> #define MXS_SPI_PORT_OFFSET 0x2000 > > > >> > > > >> -#define MXS_SSP_CHIPSELECT_MASK 0x00300000 > > > >> +#define MXS_SSP_CHIPSELECT_MASK GENMASK(21, 20) > > > >> > > > >> #define MXS_SSP_CHIPSELECT_SHIFT 20 > > > > > > > > This is just making things unreadable, please keep it as is. NAK. > > > > > > What's wrong with the GENMASK here is that something that you against > > > with it? It don't look like unreadable. > > > > If I open the datasheet, I can easily locate mask 0x0030_0000 and figure > > out which bits I need to work with. With genmask ... not so much. It only > > obfuscates the code. > > Really? I don't have the "mxs" datasheet handy but I have the mx6 > solo/duallite one handy and the SPI chapter talks about bits and has > them broken down that way, not the hex numbers for masking whatever > field. This matched my expectation on how I recall the TI parts being > as well, bit field descriptions and binary values, not hex. MXS is sigmatel design, so the datasheets are different. And I am much more fond of a bitmask being a bitmask (or a hex number) than some ad-hoc macro. Best regards, Marek Vasut
On Sun, Oct 25, 2015 at 12:12:12AM +0200, Marek Vasut wrote: > On Saturday, October 24, 2015 at 11:49:43 PM, Tom Rini wrote: > > On Sat, Oct 24, 2015 at 03:48:14PM +0200, Marek Vasut wrote: > > > On Saturday, October 24, 2015 at 03:42:43 PM, Jagan Teki wrote: > > > > On 24 October 2015 at 18:10, Marek Vasut <marex@denx.de> wrote: > > > > > On Saturday, October 24, 2015 at 05:39:07 AM, Jagan Teki wrote: > > > > >> Replace numeric mask hexcodes with GENMASK macro > > > > >> in mxs_spi > > > > >> > > > > >> Cc: Marek Vasut <marex@denx.de> > > > > >> Signed-off-by: Jagan Teki <jteki@openedev.com> > > > > >> --- > > > > >> > > > > >> 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 @@ > > > > >> > > > > >> #define MXS_SPI_MAX_TIMEOUT 1000000 > > > > >> #define MXS_SPI_PORT_OFFSET 0x2000 > > > > >> > > > > >> -#define MXS_SSP_CHIPSELECT_MASK 0x00300000 > > > > >> +#define MXS_SSP_CHIPSELECT_MASK GENMASK(21, 20) > > > > >> > > > > >> #define MXS_SSP_CHIPSELECT_SHIFT 20 > > > > > > > > > > This is just making things unreadable, please keep it as is. NAK. > > > > > > > > What's wrong with the GENMASK here is that something that you against > > > > with it? It don't look like unreadable. > > > > > > If I open the datasheet, I can easily locate mask 0x0030_0000 and figure > > > out which bits I need to work with. With genmask ... not so much. It only > > > obfuscates the code. > > > > Really? I don't have the "mxs" datasheet handy but I have the mx6 > > solo/duallite one handy and the SPI chapter talks about bits and has > > them broken down that way, not the hex numbers for masking whatever > > field. This matched my expectation on how I recall the TI parts being > > as well, bit field descriptions and binary values, not hex. > > MXS is sigmatel design, so the datasheets are different. And I am much more > fond of a bitmask being a bitmask (or a hex number) than some ad-hoc macro. Oh yeah, those parts. I don't have my sigmatel copy of those datasheets around anymore either, heh. I'm still used to datasheets that talk in terms of bitfields and binary and not giving you the hex mask.
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 @@ #define MXS_SPI_MAX_TIMEOUT 1000000 #define MXS_SPI_PORT_OFFSET 0x2000 -#define MXS_SSP_CHIPSELECT_MASK 0x00300000 +#define MXS_SSP_CHIPSELECT_MASK GENMASK(21, 20) #define MXS_SSP_CHIPSELECT_SHIFT 20 #define MXSSSP_SMALL_TRANSFER 512
Replace numeric mask hexcodes with GENMASK macro in mxs_spi Cc: Marek Vasut <marex@denx.de> Signed-off-by: Jagan Teki <jteki@openedev.com> --- drivers/spi/mxs_spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)