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

[Matthias Brugger <matthias.bgg@gmail.com>]

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@mediatek.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)
>