linux-kernel - Re: [PATCH v8 2/3] PCI: rockchip: Simplify reset control handling by using reset_control

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: <20241015125945.llbyg6w7viucw5rh@thinkpad>
Date: Tue, 15 Oct 2024 18:29:45 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: Anand Moon <linux.amoon@...il.com>
Cc: Shawn Lin <shawn.lin@...k-chips.com>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Heiko Stuebner <heiko@...ech.de>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	"open list:PCIE DRIVER FOR ROCKCHIP" <linux-pci@...r.kernel.org>,
	"open list:PCIE DRIVER FOR ROCKCHIP" <linux-rockchip@...ts.infradead.org>,
	"moderated list:ARM/Rockchip SoC support" <linux-arm-kernel@...ts.infradead.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 2/3] PCI: rockchip: Simplify reset control handling by
 using reset_control_bulk*() function

On Tue, Oct 15, 2024 at 02:30:23PM +0530, Anand Moon wrote:
> Hi Manivannan,
> 
> Thanks for your review comments.
> 
> On Tue, 15 Oct 2024 at 10:41, Manivannan Sadhasivam
> <manivannan.sadhasivam@...aro.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..
> 

How about,

"Currently, the driver acquires and asserts/deasserts the resets individually
thereby making the driver complex to read. But this can be simplified by using
the reset_control_bulk APIs. Use devm_reset_control_bulk_get_exclusive() API to
acquire all the resets and use reset_control_bulk_{assert/deassert}() APIs to
assert/deassert them in bulk.

Also follow the recommendations provided in 'Rockchip RK3399 TRM v1.3 Part2':
..."

- Mani

> 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@...il.com>
> >
> > Some nitpicks below. Rest looks good.
> 
> I will fix these in the next version.
> 
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்
> 
> Thanks
> -Anand

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