Re: [PATCH] XOR uses

2015-12-08 Thread Michael McConville
Marcelo Araujo wrote:
> What would be the advantage of it?

Just clarity and readability.

> I still prefer explicit than implicit. The current code is much more
> readable.

I very much disagree. XOR is a basic binary operation, like AND and OR.
I don't understand what's more explicit about using 4x the code to
reimplement it every time. I don't feel strongly about the patches fate,
though.

> 2015-12-08 11:13 GMT+08:00 Michael McConville :
> 
> > A minor simplification patch:
> >
> >
> > Index: sys/arm/allwinner/a10_gpio.c
> > ===
> > --- sys/arm/allwinner/a10_gpio.c(revision 291978)
> > +++ sys/arm/allwinner/a10_gpio.c(working copy)
> > @@ -356,10 +356,7 @@
> > sc = device_get_softc(dev);
> > A10_GPIO_LOCK(sc);
> > data = A10_GPIO_READ(sc, A10_GPIO_GP_DAT(bank));
> > -   if (data & (1 << pin))
> > -   data &= ~(1 << pin);
> > -   else
> > -   data |= (1 << pin);
> > +   data ^= (1 << pin);
> > A10_GPIO_WRITE(sc, A10_GPIO_GP_DAT(bank), data);
> > A10_GPIO_UNLOCK(sc);
> >
> > Index: sys/arm/altera/socfpga/socfpga_gpio.c
> > ===
> > --- sys/arm/altera/socfpga/socfpga_gpio.c   (revision 291978)
> > +++ sys/arm/altera/socfpga/socfpga_gpio.c   (working copy)
> > @@ -336,10 +336,7 @@
> >
> > GPIO_LOCK(sc);
> > reg = READ4(sc, GPIO_SWPORTA_DR);
> > -   if (reg & (1 << i))
> > -   reg &= ~(1 << i);
> > -   else
> > -   reg |= (1 << i);
> > +   reg ^= (1 << i);
> > WRITE4(sc, GPIO_SWPORTA_DR, reg);
> > GPIO_UNLOCK(sc);
> >
> > Index: sys/arm/rockchip/rk30xx_gpio.c
> > ===
> > --- sys/arm/rockchip/rk30xx_gpio.c  (revision 291978)
> > +++ sys/arm/rockchip/rk30xx_gpio.c  (working copy)
> > @@ -375,10 +375,7 @@
> > return (EINVAL);
> > RK30_GPIO_LOCK(sc);
> > data = RK30_GPIO_READ(sc, RK30_GPIO_SWPORT_DR);
> > -   if (data & (1U << pin))
> > -   data &= ~(1U << pin);
> > -   else
> > -   data |= (1U << pin);
> > +   data ^= (1U << pin);
> > RK30_GPIO_WRITE(sc, RK30_GPIO_SWPORT_DR, data);
> > RK30_GPIO_UNLOCK(sc);
> >
> > Index: sys/arm/samsung/exynos/exynos5_pad.c
> > ===
> > --- sys/arm/samsung/exynos/exynos5_pad.c(revision 291978)
> > +++ sys/arm/samsung/exynos/exynos5_pad.c(working copy)
> > @@ -722,10 +722,7 @@
> >
> > GPIO_LOCK(sc);
> > reg = READ4(sc, bank.port, bank.con + 0x4);
> > -   if (reg & (1 << pin_shift))
> > -   reg &= ~(1 << pin_shift);
> > -   else
> > -   reg |= (1 << pin_shift);
> > +   reg ^= (1 << pin_shift);
> > WRITE4(sc, bank.port, bank.con + 0x4, reg);
> > GPIO_UNLOCK(sc);
> >
> > Index: sys/dev/nand/nandsim_ctrl.c
> > ===
> > --- sys/dev/nand/nandsim_ctrl.c (revision 291978)
> > +++ sys/dev/nand/nandsim_ctrl.c (working copy)
> > @@ -388,9 +388,6 @@
> > rand = random();
> > if ((rand % 100) < chip->error_ratio) {
> > bit = rand % 8;
> > -   if (*byte & (1 << bit))
> > -   *byte &= ~(1 << bit);
> > -   else
> > -   *byte |= (1 << bit);
> > +   *byte ^= (1 << bit);
> > }
> >  }
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: [PATCH] XOR uses

2015-12-08 Thread Hans Petter Selasky

On 12/08/15 08:57, Ranjan1018 . wrote:

Hi,

I prefer the syntax in the patch:
- the semantic is more clear for me: if you want to flip a bit you should
use xor
- it saves, probably, some bytes of assembly code

Regards
Maurizio


+1

And it is unconditional.

--HPS
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: [PATCH] XOR uses

2015-12-07 Thread Ranjan1018 .
2015-12-08 5:40 GMT+01:00 Marcelo Araujo :

> Hi Michael,
>
> What would be the advantage of it?
> I still prefer explicit than implicit. The current code is much more
> readable.
>
> Best,
>
> 2015-12-08 11:13 GMT+08:00 Michael McConville :
>
> > A minor simplification patch:
> >
> >
> > Index: sys/arm/allwinner/a10_gpio.c
> > ===
> > --- sys/arm/allwinner/a10_gpio.c(revision 291978)
> > +++ sys/arm/allwinner/a10_gpio.c(working copy)
> > @@ -356,10 +356,7 @@
> > sc = device_get_softc(dev);
> > A10_GPIO_LOCK(sc);
> > data = A10_GPIO_READ(sc, A10_GPIO_GP_DAT(bank));
> > -   if (data & (1 << pin))
> > -   data &= ~(1 << pin);
> > -   else
> > -   data |= (1 << pin);
> > +   data ^= (1 << pin);
> > A10_GPIO_WRITE(sc, A10_GPIO_GP_DAT(bank), data);
> > A10_GPIO_UNLOCK(sc);
> >
> > Index: sys/arm/altera/socfpga/socfpga_gpio.c
> > ===
> > --- sys/arm/altera/socfpga/socfpga_gpio.c   (revision 291978)
> > +++ sys/arm/altera/socfpga/socfpga_gpio.c   (working copy)
> > @@ -336,10 +336,7 @@
> >
> > GPIO_LOCK(sc);
> > reg = READ4(sc, GPIO_SWPORTA_DR);
> > -   if (reg & (1 << i))
> > -   reg &= ~(1 << i);
> > -   else
> > -   reg |= (1 << i);
> > +   reg ^= (1 << i);
> > WRITE4(sc, GPIO_SWPORTA_DR, reg);
> > GPIO_UNLOCK(sc);
> >
> > Index: sys/arm/rockchip/rk30xx_gpio.c
> > ===
> > --- sys/arm/rockchip/rk30xx_gpio.c  (revision 291978)
> > +++ sys/arm/rockchip/rk30xx_gpio.c  (working copy)
> > @@ -375,10 +375,7 @@
> > return (EINVAL);
> > RK30_GPIO_LOCK(sc);
> > data = RK30_GPIO_READ(sc, RK30_GPIO_SWPORT_DR);
> > -   if (data & (1U << pin))
> > -   data &= ~(1U << pin);
> > -   else
> > -   data |= (1U << pin);
> > +   data ^= (1U << pin);
> > RK30_GPIO_WRITE(sc, RK30_GPIO_SWPORT_DR, data);
> > RK30_GPIO_UNLOCK(sc);
> >
> > Index: sys/arm/samsung/exynos/exynos5_pad.c
> > ===
> > --- sys/arm/samsung/exynos/exynos5_pad.c(revision 291978)
> > +++ sys/arm/samsung/exynos/exynos5_pad.c(working copy)
> > @@ -722,10 +722,7 @@
> >
> > GPIO_LOCK(sc);
> > reg = READ4(sc, bank.port, bank.con + 0x4);
> > -   if (reg & (1 << pin_shift))
> > -   reg &= ~(1 << pin_shift);
> > -   else
> > -   reg |= (1 << pin_shift);
> > +   reg ^= (1 << pin_shift);
> > WRITE4(sc, bank.port, bank.con + 0x4, reg);
> > GPIO_UNLOCK(sc);
> >
> > Index: sys/dev/nand/nandsim_ctrl.c
> > ===
> > --- sys/dev/nand/nandsim_ctrl.c (revision 291978)
> > +++ sys/dev/nand/nandsim_ctrl.c (working copy)
> > @@ -388,9 +388,6 @@
> > rand = random();
> > if ((rand % 100) < chip->error_ratio) {
> > bit = rand % 8;
> > -   if (*byte & (1 << bit))
> > -   *byte &= ~(1 << bit);
> > -   else
> > -   *byte |= (1 << bit);
> > +   *byte ^= (1 << bit);
> > }
> >  }
> >
>

Hi,

I prefer the syntax in the patch:
- the semantic is more clear for me: if you want to flip a bit you should
use xor
- it saves, probably, some bytes of assembly code

Regards
Maurizio
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: [PATCH] XOR uses

2015-12-07 Thread Marcelo Araujo
Hi Michael,

What would be the advantage of it?
I still prefer explicit than implicit. The current code is much more
readable.

Best,

2015-12-08 11:13 GMT+08:00 Michael McConville :

> A minor simplification patch:
>
>
> Index: sys/arm/allwinner/a10_gpio.c
> ===
> --- sys/arm/allwinner/a10_gpio.c(revision 291978)
> +++ sys/arm/allwinner/a10_gpio.c(working copy)
> @@ -356,10 +356,7 @@
> sc = device_get_softc(dev);
> A10_GPIO_LOCK(sc);
> data = A10_GPIO_READ(sc, A10_GPIO_GP_DAT(bank));
> -   if (data & (1 << pin))
> -   data &= ~(1 << pin);
> -   else
> -   data |= (1 << pin);
> +   data ^= (1 << pin);
> A10_GPIO_WRITE(sc, A10_GPIO_GP_DAT(bank), data);
> A10_GPIO_UNLOCK(sc);
>
> Index: sys/arm/altera/socfpga/socfpga_gpio.c
> ===
> --- sys/arm/altera/socfpga/socfpga_gpio.c   (revision 291978)
> +++ sys/arm/altera/socfpga/socfpga_gpio.c   (working copy)
> @@ -336,10 +336,7 @@
>
> GPIO_LOCK(sc);
> reg = READ4(sc, GPIO_SWPORTA_DR);
> -   if (reg & (1 << i))
> -   reg &= ~(1 << i);
> -   else
> -   reg |= (1 << i);
> +   reg ^= (1 << i);
> WRITE4(sc, GPIO_SWPORTA_DR, reg);
> GPIO_UNLOCK(sc);
>
> Index: sys/arm/rockchip/rk30xx_gpio.c
> ===
> --- sys/arm/rockchip/rk30xx_gpio.c  (revision 291978)
> +++ sys/arm/rockchip/rk30xx_gpio.c  (working copy)
> @@ -375,10 +375,7 @@
> return (EINVAL);
> RK30_GPIO_LOCK(sc);
> data = RK30_GPIO_READ(sc, RK30_GPIO_SWPORT_DR);
> -   if (data & (1U << pin))
> -   data &= ~(1U << pin);
> -   else
> -   data |= (1U << pin);
> +   data ^= (1U << pin);
> RK30_GPIO_WRITE(sc, RK30_GPIO_SWPORT_DR, data);
> RK30_GPIO_UNLOCK(sc);
>
> Index: sys/arm/samsung/exynos/exynos5_pad.c
> ===
> --- sys/arm/samsung/exynos/exynos5_pad.c(revision 291978)
> +++ sys/arm/samsung/exynos/exynos5_pad.c(working copy)
> @@ -722,10 +722,7 @@
>
> GPIO_LOCK(sc);
> reg = READ4(sc, bank.port, bank.con + 0x4);
> -   if (reg & (1 << pin_shift))
> -   reg &= ~(1 << pin_shift);
> -   else
> -   reg |= (1 << pin_shift);
> +   reg ^= (1 << pin_shift);
> WRITE4(sc, bank.port, bank.con + 0x4, reg);
> GPIO_UNLOCK(sc);
>
> Index: sys/dev/nand/nandsim_ctrl.c
> ===
> --- sys/dev/nand/nandsim_ctrl.c (revision 291978)
> +++ sys/dev/nand/nandsim_ctrl.c (working copy)
> @@ -388,9 +388,6 @@
> rand = random();
> if ((rand % 100) < chip->error_ratio) {
> bit = rand % 8;
> -   if (*byte & (1 << bit))
> -   *byte &= ~(1 << bit);
> -   else
> -   *byte |= (1 << bit);
> +   *byte ^= (1 << bit);
> }
>  }
> ___
> freebsd-current@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
>



-- 

-- 
Marcelo Araujo(__)ara...@freebsd.org
\\\'',)http://www.FreeBSD.org    \/  \ ^
Power To Server. .\. /_)
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"