lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 16 Jan 2017 15:58:36 +0800
From:   Shawn Guo <shawnguo@...nel.org>
To:     Baoyou Xie <baoyou.xie@...aro.org>
Cc:     jun.nie@...aro.org, p.zabel@...gutronix.de, robh+dt@...nel.org,
        mark.rutland@....com, linux-arm-kernel@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        xie.baoyou@....com.cn, chen.chaokai@....com.cn,
        wang.qiang01@....com.cn
Subject: Re: [PATCH v1 3/3] reset: zx2967: add reset controller driver for
 ZTE's zx2967 family

On Sat, Jan 14, 2017 at 03:05:30PM +0800, Baoyou Xie wrote:
> +static int zx2967_reset_assert(struct reset_controller_dev *rcdev,
> +			   unsigned long id)
> +{
> +	struct zx2967_reset *reset = NULL;
> +	int bank = id / 32;
> +	int offset = id % 32;
> +	unsigned int reg;

u32 is probably better for register value.

> +	unsigned long flags;
> +
> +	reset = container_of(rcdev, struct zx2967_reset, rcdev);
> +
> +	spin_lock_irqsave(&reset->lock, flags);
> +
> +	reg = readl(reset->reg_base + (bank * 4));
> +	writel(reg & ~BIT(offset), reset->reg_base + (bank * 4));
> +	reg = readl(reset->reg_base + (bank * 4));

Is this read on the register is necessary?  If so, we should probably
have a comment for that.

> +
> +	spin_unlock_irqrestore(&reset->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int zx2967_reset_deassert(struct reset_controller_dev *rcdev,
> +			     unsigned long id)

Please indent the line right after parentheses.

> +{
> +	struct zx2967_reset *reset = NULL;
> +	int bank = id / 32;
> +	int offset = id % 32;
> +	unsigned int reg;
> +	unsigned long flags;
> +
> +	reset = container_of(rcdev, struct zx2967_reset, rcdev);
> +
> +	spin_lock_irqsave(&reset->lock, flags);
> +
> +	reg = readl(reset->reg_base + (bank * 4));
> +	writel(reg | BIT(offset), reset->reg_base + (bank * 4));
> +	reg = readl(reset->reg_base + (bank * 4));
> +
> +	spin_unlock_irqrestore(&reset->lock, flags);
> +
> +	return 0;
> +}

Only difference between these two functions is only one line.  Should we
consolidate them a bit?

Shawn

Powered by blists - more mailing lists