linux-kernel - Re: [PATCH 1/2] reset-controller: ti: adjust the reset assert and deassert interface

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]

Message-ID: <ba0d1e29-3ba3-5379-d03e-1ccec21c2ffa@gmail.com>
Date:   Wed, 29 Jul 2020 09:48:42 +0200
From:   Matthias Brugger <matthias.bgg@...il.com>
To:     Crystal Guo <crystal.guo@...iatek.com>, p.zabel@...gutronix.de,
        robh+dt@...nel.org
Cc:     srv_heupstream@...iatek.com, linux-mediatek@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, seiya.wang@...iatek.com
Subject: Re: [PATCH 1/2] reset-controller: ti: adjust the reset assert and
 deassert interface



On 29/07/2020 09:39, Crystal Guo wrote:
> Add ti_syscon_reset() to integrate assert and deassert together,
> and change return value of the reset assert and deassert interface
> from regmap_update_bits to regmap_write_bits.
> 
> when clear bit is already 1, regmap_update_bits can not write 1 to it again.
> Some IC has the feature that, when set bit is 1, the clear bit change
> to 1 together. It will truly clear bit to 0 by write 1 to the clear bit
> 
> Signed-off-by: Crystal Guo <crystal.guo@...iatek.com>
> ---
>   drivers/reset/reset-ti-syscon.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c
> index a2635c2..5a8ec8f 100644
> --- a/drivers/reset/reset-ti-syscon.c
> +++ b/drivers/reset/reset-ti-syscon.c
> @@ -89,7 +89,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);

Nack, this will break the driver for the other devices.
The kernel has to work not just for your SoC but for all devices of all 
architectures. You can't just hack something up, that will work on your specific 
SoC.

Regards,
Matthias

>   }
>   
>   /**
> @@ -120,7 +120,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);
>   }
>   
>   /**
> @@ -158,10 +158,19 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev,
>   		!(control->flags & STATUS_SET);
>   }
>   
> +static int ti_syscon_reset(struct reset_controller_dev *rcdev,
> +			   unsigned long id)
> +{
> +	ti_syscon_reset_assert(rcdev, id);
> +
> +	return ti_syscon_reset_deassert(rcdev, id);
> +}
> +
>   static const struct reset_control_ops ti_syscon_reset_ops = {
>   	.assert		= ti_syscon_reset_assert,
>   	.deassert	= ti_syscon_reset_deassert,
>   	.status		= ti_syscon_reset_status,
> +	.reset		= ti_syscon_reset,
>   };
>   
>   static int ti_syscon_reset_probe(struct platform_device *pdev)
>