| Message ID | 20241012050611.1908-4-linux.amoon@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCIe RK3399 clock and reset using new helper functions | expand |
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
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);