Re: [PATCH v8 2/3] PCI: rockchip: Simplify reset control handling by using reset_control_bulk*() function

[Anand Moon <linux.amoon@gmail.com>]

Hi Manivannan,

Thanks for your review comments.

On Tue, 15 Oct 2024 at 10:41, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Mon, Oct 14, 2024 at 07:22:03PM +0530, Anand Moon wrote:
> > Refactor the reset control handling in the Rockchip PCIe driver,
> > introduce a more robust and efficient method for assert and
> > deassert reset controller using reset_control_bulk*() API. Using the
> > reset_control_bulk APIs, the reset handling for the core clocks reset
> > unit becomes much simpler.
> >
> > Spilt the reset controller in two groups as per the
> >  RK3399 TM  17.5.8.1 PCIe Initialization Sequence
> >     17.5.8.1.1 PCIe as Root Complex.
> >
> > 6. De-assert the PIPE_RESET_N/MGMT_STICKY_RESET_N/MGMT_RESET_N/RESET_N
> >    simultaneously.
> >
>
> I'd reword it slightly:
>
> Following the recommendations in 'Rockchip RK3399 TRM v1.3 Part2':
>
> 1. Split the reset controls into two groups as per section '17.5.8.1.1 PCIe
> as Root Complex'.
>
> 2. Deassert the 'Pipe, MGMT Sticky, MGMT, Core' resets in groups as per section
> '17.5.8.1.1 PCIe as Root Complex'. This is accomplished using the
> reset_control_bulk APIs.
>
> > - devm_reset_control_bulk_get_exclusive(): Allows the driver to get all
> >   resets defined in the DT thereby removing the hardcoded reset names
> >   in the driver.
> > - reset_control_bulk_assert(): Allows the driver to assert the resets
> >   defined in the driver.
> > - reset_control_bulk_deassert(): Allows the driver to deassert the resets
> >   defined in the driver.
> >
>
> No need to list out the APIs. Just add them to the first paragraph itself to
> explain how they are used.
>

Here is a short version of the commit message.

Introduce a more robust and efficient method for assert and deassert
the reset controller using the reset_control_bulk*() API.
Simplify reset handling for the core clocks reset unit with the
reset_control_bulk APIs.

devm_reset_control_bulk_get_exclusive(): Obtain all resets from the
            device tree, removing hardcoded names.
reset_control_bulk_assert(): assert the resets defined in the driver.
reset_control_bulk_deassert(): deassert the resets defined in the driver..

Following the recommendations in 'Rockchip RK3399 TRM v1.3 Part2':

1. Split the reset controls into two groups as per section '17.5.8.1.1 PCIe
as Root Complex'.

2. Deassert the 'Pipe, MGMT Sticky, MGMT, Core' resets in groups as per section
'17.5.8.1.1 PCIe as Root Complex'. This is accomplished using the
reset_control_bulk APIs.

Does this look good to you? Let me know if you need any further adjustments!

I will fix this for CLK bulk as well.

> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>
> Some nitpicks below. Rest looks good.

I will fix these in the next version.

> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்

Thanks
-Anand