Refactor the rockchip_pcie_disable_clocks function to accept a
struct rockchip_pcie pointer instead of a void pointer. This change
improves type safety and code readability by explicitly specifying
the expected data type.

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v7: None
v6: Fix the subject, add the missing () in the function name.
v5: Fix the commit message and add r-b Manivannan.
v4: None
v3: None
v2: No
---
 drivers/pci/controller/pcie-rockchip.c | 3 +--
 drivers/pci/controller/pcie-rockchip.h | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
index 9a118e2b8cbd..c3147111f1a7 100644
--- a/drivers/pci/controller/pcie-rockchip.c
+++ b/drivers/pci/controller/pcie-rockchip.c
@@ -269,9 +269,8 @@ int rockchip_pcie_enable_clocks(struct rockchip_pcie *rockchip)
 }
 EXPORT_SYMBOL_GPL(rockchip_pcie_enable_clocks);
 
-void rockchip_pcie_disable_clocks(void *data)
+void rockchip_pcie_disable_clocks(struct rockchip_pcie *rockchip)
 {
-	struct rockchip_pcie *rockchip = data;
 
 	clk_bulk_disable_unprepare(rockchip->num_clks, rockchip->clks);
 }
diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
index 2761699f670b..7f0f938e9195 100644
--- a/drivers/pci/controller/pcie-rockchip.h
+++ b/drivers/pci/controller/pcie-rockchip.h
@@ -347,7 +347,7 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip);
 int rockchip_pcie_get_phys(struct rockchip_pcie *rockchip);
 void rockchip_pcie_deinit_phys(struct rockchip_pcie *rockchip);
 int rockchip_pcie_enable_clocks(struct rockchip_pcie *rockchip);
-void rockchip_pcie_disable_clocks(void *data);
+void rockchip_pcie_disable_clocks(struct rockchip_pcie *rockchip);
 void rockchip_pcie_cfg_configuration_accesses(
 		struct rockchip_pcie *rockchip, u32 type);
 
-- 
2.44.0

On Sat, Oct 12, 2024 at 10:36:05AM +0530, Anand Moon wrote:
> Refactor the rockchip_pcie_disable_clocks function to accept a
> struct rockchip_pcie pointer instead of a void pointer. This change
> improves type safety and code readability by explicitly specifying
> the expected data type.
> 
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> v7: None
> v6: Fix the subject, add the missing () in the function name.

Did you remove it in v7? Please don't do that, it just increases the burden on
reviewers.

- Mani

> v5: Fix the commit message and add r-b Manivannan.
> v4: None
> v3: None
> v2: No
> ---
>  drivers/pci/controller/pcie-rockchip.c | 3 +--
>  drivers/pci/controller/pcie-rockchip.h | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
> index 9a118e2b8cbd..c3147111f1a7 100644
> --- a/drivers/pci/controller/pcie-rockchip.c
> +++ b/drivers/pci/controller/pcie-rockchip.c
> @@ -269,9 +269,8 @@ int rockchip_pcie_enable_clocks(struct rockchip_pcie *rockchip)
>  }
>  EXPORT_SYMBOL_GPL(rockchip_pcie_enable_clocks);
>  
> -void rockchip_pcie_disable_clocks(void *data)
> +void rockchip_pcie_disable_clocks(struct rockchip_pcie *rockchip)
>  {
> -	struct rockchip_pcie *rockchip = data;
>  
>  	clk_bulk_disable_unprepare(rockchip->num_clks, rockchip->clks);
>  }
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index 2761699f670b..7f0f938e9195 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -347,7 +347,7 @@ int rockchip_pcie_init_port(struct rockchip_pcie *rockchip);
>  int rockchip_pcie_get_phys(struct rockchip_pcie *rockchip);
>  void rockchip_pcie_deinit_phys(struct rockchip_pcie *rockchip);
>  int rockchip_pcie_enable_clocks(struct rockchip_pcie *rockchip);
> -void rockchip_pcie_disable_clocks(void *data);
> +void rockchip_pcie_disable_clocks(struct rockchip_pcie *rockchip);
>  void rockchip_pcie_cfg_configuration_accesses(
>  		struct rockchip_pcie *rockchip, u32 type);
>  
> -- 
> 2.44.0
> 

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

Hi Manivannan,

Thanks for your review comments.

On Sat, 12 Oct 2024 at 11:50, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Sat, Oct 12, 2024 at 10:36:05AM +0530, Anand Moon wrote:
> > Refactor the rockchip_pcie_disable_clocks function to accept a
> > struct rockchip_pcie pointer instead of a void pointer. This change
> > improves type safety and code readability by explicitly specifying
> > the expected data type.
> >
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > v7: None
> > v6: Fix the subject, add the missing () in the function name.
>
> Did you remove it in v7? Please don't do that, it just increases the burden on
> reviewers.
>
> - Mani
Earlier, it was reported that function () should be used in the function name.

Thanks

-Anand

On Sat, Oct 12, 2024 at 12:55:38PM +0530, Anand Moon wrote:
> Hi Manivannan,
> 
> Thanks for your review comments.
> 
> On Sat, 12 Oct 2024 at 11:50, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Sat, Oct 12, 2024 at 10:36:05AM +0530, Anand Moon wrote:
> > > Refactor the rockchip_pcie_disable_clocks function to accept a
> > > struct rockchip_pcie pointer instead of a void pointer. This change
> > > improves type safety and code readability by explicitly specifying
> > > the expected data type.
> > >
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > > v7: None
> > > v6: Fix the subject, add the missing () in the function name.
> >
> > Did you remove it in v7? Please don't do that, it just increases the burden on
> > reviewers.
> >
> > - Mani
> Earlier, it was reported that function () should be used in the function name.

Hmm. Why can't you do the same for the description also?

- Mani

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

Hi Manivannan,

On Sat, 12 Oct 2024 at 13:32, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Sat, Oct 12, 2024 at 12:55:38PM +0530, Anand Moon wrote:
> > Hi Manivannan,
> >
> > Thanks for your review comments.
> >
> > On Sat, 12 Oct 2024 at 11:50, Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > On Sat, Oct 12, 2024 at 10:36:05AM +0530, Anand Moon wrote:
> > > > Refactor the rockchip_pcie_disable_clocks function to accept a
> > > > struct rockchip_pcie pointer instead of a void pointer. This change
> > > > improves type safety and code readability by explicitly specifying
> > > > the expected data type.
> > > >
> > > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > > ---
> > > > v7: None
> > > > v6: Fix the subject, add the missing () in the function name.
> > >
> > > Did you remove it in v7? Please don't do that, it just increases the burden on
> > > reviewers.
> > >
> > > - Mani
> > Earlier, it was reported that function () should be used in the function name.
>
> Hmm. Why can't you do the same for the description also?
>
Ok, I missed this point.
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்

Thanks

-Anand