linux-kernel - Re: [v6, 3/3] reset-controller: ti: force the write operation when assert or deassert

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Nov 2020 19:13:40 +0800
From:   Ikjoon Jang <ikjn@...omium.org>
To:     Crystal Guo <crystal.guo@...iatek.com>
Cc:     p.zabel@...gutronix.de, robh+dt@...nel.org, matthias.bgg@...il.com,
        devicetree@...r.kernel.org, yong.liang@...iatek.com,
        stanley.chu@...iatek.com, srv_heupstream@...iatek.com,
        seiya.wang@...iatek.com, linux-kernel@...r.kernel.org,
        fan.chen@...iatek.com, linux-mediatek@...ts.infradead.org,
        yingjoe.chen@...iatek.com, s-anna@...com,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [v6, 3/3] reset-controller: ti: force the write operation when
 assert or deassert

On Wed, Sep 30, 2020 at 10:21:59AM +0800, Crystal Guo wrote:
> Force the write operation in case the read already happens
> to return the correct value.
> 
> Signed-off-by: Crystal Guo <crystal.guo@...iatek.com>
> ---
>  drivers/reset/reset-ti-syscon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c
> index 5d1f8306cd4f..c34394f1e9e2 100644
> --- a/drivers/reset/reset-ti-syscon.c
> +++ b/drivers/reset/reset-ti-syscon.c
> @@ -97,7 +97,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev,
>  	mask = BIT(control->assert_bit);
>  	value = (control->flags & ASSERT_SET) ? mask : 0x0;
>  
> -	return regmap_update_bits(data->regmap, control->assert_offset, mask, value);
> +	return regmap_write_bits(data->regmap, control->assert_offset, mask, value);
>  }

I don't think there are no reset controllers with cached regmap,
thus I don't think this is needed.
Are there any specific reasons behind this, what I've missed here?

We need to be sure that all other devices using this driver
should have no side effects on write.

I can think of a weird controller doing unwanted things internally
on every write disregarding the current state. (or is this overly
paranoid?)

>  
>  /**
> @@ -128,7 +128,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev,
>  	mask = BIT(control->deassert_bit);
>  	value = (control->flags & DEASSERT_SET) ? mask : 0x0;
>  
> -	return regmap_update_bits(data->regmap, control->deassert_offset, mask, value);
> +	return regmap_write_bits(data->regmap, control->deassert_offset, mask, value);
>  }
>  
>  /**