Currently, the driver acquires and asserts/deasserts the resets
individually thereby making the driver complex to read.
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.
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
drivers/pci/controller/dwc/pcie-histb.c | 57 ++++++++++++-------------
1 file changed, 28 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-histb.c b/drivers/pci/controller/dwc/pcie-histb.c
index 4022349e85d2..4ba5c9af63a0 100644
--- a/drivers/pci/controller/dwc/pcie-histb.c
+++ b/drivers/pci/controller/dwc/pcie-histb.c
@@ -49,14 +49,20 @@
#define PCIE_LTSSM_STATE_MASK GENMASK(5, 0)
#define PCIE_LTSSM_STATE_ACTIVE 0x11
+#define PCIE_HISTB_NUM_RESETS ARRAY_SIZE(histb_pci_rsts)
+
+static const char * const histb_pci_rsts[] = {
+ "soft",
+ "sys",
+ "bus",
+};
+
struct histb_pcie {
struct dw_pcie *pci;
struct clk_bulk_data *clks;
int num_clks;
struct phy *phy;
- struct reset_control *soft_reset;
- struct reset_control *sys_reset;
- struct reset_control *bus_reset;
+ struct reset_control_bulk_data reset[PCIE_HISTB_NUM_RESETS];
void __iomem *ctrl;
struct gpio_desc *reset_gpio;
struct regulator *vpcie;
@@ -198,9 +204,8 @@ static const struct dw_pcie_host_ops histb_pcie_host_ops = {
static void histb_pcie_host_disable(struct histb_pcie *hipcie)
{
- reset_control_assert(hipcie->soft_reset);
- reset_control_assert(hipcie->sys_reset);
- reset_control_assert(hipcie->bus_reset);
+ reset_control_bulk_assert(PCIE_HISTB_NUM_RESETS,
+ hipcie->reset);
clk_bulk_disable_unprepare(hipcie->num_clks, hipcie->clks);
@@ -236,14 +241,19 @@ static int histb_pcie_host_enable(struct dw_pcie_rp *pp)
goto reg_dis;
}
- reset_control_assert(hipcie->soft_reset);
- reset_control_deassert(hipcie->soft_reset);
-
- reset_control_assert(hipcie->sys_reset);
- reset_control_deassert(hipcie->sys_reset);
+ ret = reset_control_bulk_assert(PCIE_HISTB_NUM_RESETS,
+ hipcie->reset);
+ if (ret) {
+ dev_err(dev, "Couldn't assert reset %d\n", ret);
+ goto reg_dis;
+ }
- reset_control_assert(hipcie->bus_reset);
- reset_control_deassert(hipcie->bus_reset);
+ ret = reset_control_bulk_deassert(PCIE_HISTB_NUM_RESETS,
+ hipcie->reset);
+ if (ret) {
+ dev_err(dev, "Couldn't dessert reset %d\n", ret);
+ goto reg_dis;
+ }
return 0;
@@ -321,23 +331,12 @@ static int histb_pcie_probe(struct platform_device *pdev)
return dev_err_probe(dev, hipcie->num_clks,
"failed to get clocks\n");
- hipcie->soft_reset = devm_reset_control_get(dev, "soft");
- if (IS_ERR(hipcie->soft_reset)) {
- dev_err(dev, "couldn't get soft reset\n");
- return PTR_ERR(hipcie->soft_reset);
- }
+ ret = devm_reset_control_bulk_get_exclusive(dev,
+ PCIE_HISTB_NUM_RESETS,
+ hipcie->reset);
+ if (ret)
+ return dev_err_probe(dev, ret, "Cannot get the Core resets\n");
- hipcie->sys_reset = devm_reset_control_get(dev, "sys");
- if (IS_ERR(hipcie->sys_reset)) {
- dev_err(dev, "couldn't get sys reset\n");
- return PTR_ERR(hipcie->sys_reset);
- }
-
- hipcie->bus_reset = devm_reset_control_get(dev, "bus");
- if (IS_ERR(hipcie->bus_reset)) {
- dev_err(dev, "couldn't get bus reset\n");
- return PTR_ERR(hipcie->bus_reset);
- }
hipcie->phy = devm_phy_get(dev, "phy");
if (IS_ERR(hipcie->phy)) {
--
2.50.1In subject, remove "dwc: " to follow historical convention.
On Tue, Aug 26, 2025 at 05:12:41PM +0530, Anand Moon wrote:
> Currently, the driver acquires and asserts/deasserts the resets
> individually thereby making the driver complex to read.
>
> 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.
Please include a note that this changes the order of reset assert and
deassert and explain why this is safe.
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> drivers/pci/controller/dwc/pcie-histb.c | 57 ++++++++++++-------------
> 1 file changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-histb.c b/drivers/pci/controller/dwc/pcie-histb.c
> index 4022349e85d2..4ba5c9af63a0 100644
> --- a/drivers/pci/controller/dwc/pcie-histb.c
> +++ b/drivers/pci/controller/dwc/pcie-histb.c
> @@ -49,14 +49,20 @@
> #define PCIE_LTSSM_STATE_MASK GENMASK(5, 0)
> #define PCIE_LTSSM_STATE_ACTIVE 0x11
>
> +#define PCIE_HISTB_NUM_RESETS ARRAY_SIZE(histb_pci_rsts)
> +
> +static const char * const histb_pci_rsts[] = {
> + "soft",
> + "sys",
> + "bus",
> +};
> +
> struct histb_pcie {
> struct dw_pcie *pci;
> struct clk_bulk_data *clks;
> int num_clks;
> struct phy *phy;
> - struct reset_control *soft_reset;
> - struct reset_control *sys_reset;
> - struct reset_control *bus_reset;
> + struct reset_control_bulk_data reset[PCIE_HISTB_NUM_RESETS];
> void __iomem *ctrl;
> struct gpio_desc *reset_gpio;
> struct regulator *vpcie;
> @@ -198,9 +204,8 @@ static const struct dw_pcie_host_ops histb_pcie_host_ops = {
>
> static void histb_pcie_host_disable(struct histb_pcie *hipcie)
> {
> - reset_control_assert(hipcie->soft_reset);
> - reset_control_assert(hipcie->sys_reset);
> - reset_control_assert(hipcie->bus_reset);
> + reset_control_bulk_assert(PCIE_HISTB_NUM_RESETS,
> + hipcie->reset);
>
> clk_bulk_disable_unprepare(hipcie->num_clks, hipcie->clks);
>
> @@ -236,14 +241,19 @@ static int histb_pcie_host_enable(struct dw_pcie_rp *pp)
> goto reg_dis;
> }
>
> - reset_control_assert(hipcie->soft_reset);
> - reset_control_deassert(hipcie->soft_reset);
> -
> - reset_control_assert(hipcie->sys_reset);
> - reset_control_deassert(hipcie->sys_reset);
> + ret = reset_control_bulk_assert(PCIE_HISTB_NUM_RESETS,
> + hipcie->reset);
> + if (ret) {
> + dev_err(dev, "Couldn't assert reset %d\n", ret);
> + goto reg_dis;
> + }
>
> - reset_control_assert(hipcie->bus_reset);
> - reset_control_deassert(hipcie->bus_reset);
> + ret = reset_control_bulk_deassert(PCIE_HISTB_NUM_RESETS,
> + hipcie->reset);
> + if (ret) {
> + dev_err(dev, "Couldn't dessert reset %d\n", ret);
s/dessert/deassert/
> + goto reg_dis;
> + }
>
> return 0;
>
> @@ -321,23 +331,12 @@ static int histb_pcie_probe(struct platform_device *pdev)
> return dev_err_probe(dev, hipcie->num_clks,
> "failed to get clocks\n");
>
> - hipcie->soft_reset = devm_reset_control_get(dev, "soft");
> - if (IS_ERR(hipcie->soft_reset)) {
> - dev_err(dev, "couldn't get soft reset\n");
> - return PTR_ERR(hipcie->soft_reset);
> - }
> + ret = devm_reset_control_bulk_get_exclusive(dev,
> + PCIE_HISTB_NUM_RESETS,
> + hipcie->reset);
> + if (ret)
> + return dev_err_probe(dev, ret, "Cannot get the Core resets\n");
>
> - hipcie->sys_reset = devm_reset_control_get(dev, "sys");
> - if (IS_ERR(hipcie->sys_reset)) {
> - dev_err(dev, "couldn't get sys reset\n");
> - return PTR_ERR(hipcie->sys_reset);
> - }
> -
> - hipcie->bus_reset = devm_reset_control_get(dev, "bus");
> - if (IS_ERR(hipcie->bus_reset)) {
> - dev_err(dev, "couldn't get bus reset\n");
> - return PTR_ERR(hipcie->bus_reset);
> - }
>
> hipcie->phy = devm_phy_get(dev, "phy");
> if (IS_ERR(hipcie->phy)) {
> --
> 2.50.1
>
Hi Bjorn,
Thanks for your review comments.
On Tue, 26 Aug 2025 at 21:55, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> In subject, remove "dwc: " to follow historical convention.
>
Ok
> On Tue, Aug 26, 2025 at 05:12:41PM +0530, Anand Moon wrote:
> > Currently, the driver acquires and asserts/deasserts the resets
> > individually thereby making the driver complex to read.
> >
> > 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.
>
> Please include a note that this changes the order of reset assert and
> deassert and explain why this is safe.
>
I feel the device tree follows the same order as defined in the array.
resets = <&crg 0x18c 6>, <&crg 0x18c 5>, <&crg 0x18c 4>;
reset-names = "soft", "sys", "bus";
Ok I will update this in the commit message.
Thanks
-Anand
[cc->to: Shawn]
On Tue, Aug 26, 2025 at 11:33:17PM +0530, Anand Moon wrote:
> On Tue, 26 Aug 2025 at 21:55, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Tue, Aug 26, 2025 at 05:12:41PM +0530, Anand Moon wrote:
> > > Currently, the driver acquires and asserts/deasserts the resets
> > > individually thereby making the driver complex to read.
> > >
> > > 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.
> >
> > Please include a note that this changes the order of reset assert and
> > deassert and explain why this is safe.
> >
> I feel the device tree follows the same order as defined in the array.
>
> resets = <&crg 0x18c 6>, <&crg 0x18c 5>, <&crg 0x18c 4>;
> reset-names = "soft", "sys", "bus";
>
> Ok I will update this in the commit message.
Following the device tree order doesn't necessarily mean that this is
safe. We know the current order in the code works. We don't know
that a different order works until that's tested and verified.
These will both need acks from Shawn (pcie-histb.c maintainer).
On Di, 2025-08-26 at 17:12 +0530, Anand Moon wrote:
> Currently, the driver acquires and asserts/deasserts the resets
> individually thereby making the driver complex to read.
>
> 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.
>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> drivers/pci/controller/dwc/pcie-histb.c | 57 ++++++++++++-------------
> 1 file changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-histb.c b/drivers/pci/controller/dwc/pcie-histb.c
> index 4022349e85d2..4ba5c9af63a0 100644
> --- a/drivers/pci/controller/dwc/pcie-histb.c
> +++ b/drivers/pci/controller/dwc/pcie-histb.c
> @@ -49,14 +49,20 @@
> #define PCIE_LTSSM_STATE_MASK GENMASK(5, 0)
> #define PCIE_LTSSM_STATE_ACTIVE 0x11
>
> +#define PCIE_HISTB_NUM_RESETS ARRAY_SIZE(histb_pci_rsts)
> +
> +static const char * const histb_pci_rsts[] = {
> + "soft",
> + "sys",
> + "bus",
> +};
> +
[...]
> @@ -236,14 +241,19 @@ static int histb_pcie_host_enable(struct dw_pcie_rp *pp)
> goto reg_dis;
> }
>
> - reset_control_assert(hipcie->soft_reset);
> - reset_control_deassert(hipcie->soft_reset);
> -
> - reset_control_assert(hipcie->sys_reset);
> - reset_control_deassert(hipcie->sys_reset);
> + ret = reset_control_bulk_assert(PCIE_HISTB_NUM_RESETS,
> + hipcie->reset);
> + if (ret) {
> + dev_err(dev, "Couldn't assert reset %d\n", ret);
> + goto reg_dis;
> + }
>
> - reset_control_assert(hipcie->bus_reset);
> - reset_control_deassert(hipcie->bus_reset);
> + ret = reset_control_bulk_deassert(PCIE_HISTB_NUM_RESETS,
Note that this changes the order of assertion/deassertion, not only
because resets lines are now switched in bulk, but also because
reset_control_bulk_deassert() deasserts the reset lines in reverse
order. So this does
If the three resets are independent and order doesn't matter,
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
regards
Philipp
Hi Philipp,
Thanks for your review comments.
On Tue, 26 Aug 2025 at 18:16, Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Di, 2025-08-26 at 17:12 +0530, Anand Moon wrote:
> > Currently, the driver acquires and asserts/deasserts the resets
> > individually thereby making the driver complex to read.
> >
> > 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.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > drivers/pci/controller/dwc/pcie-histb.c | 57 ++++++++++++-------------
> > 1 file changed, 28 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-histb.c b/drivers/pci/controller/dwc/pcie-histb.c
> > index 4022349e85d2..4ba5c9af63a0 100644
> > --- a/drivers/pci/controller/dwc/pcie-histb.c
> > +++ b/drivers/pci/controller/dwc/pcie-histb.c
> > @@ -49,14 +49,20 @@
> > #define PCIE_LTSSM_STATE_MASK GENMASK(5, 0)
> > #define PCIE_LTSSM_STATE_ACTIVE 0x11
> >
> > +#define PCIE_HISTB_NUM_RESETS ARRAY_SIZE(histb_pci_rsts)
> > +
> > +static const char * const histb_pci_rsts[] = {
> > + "soft",
> > + "sys",
> > + "bus",
> > +};
> > +
> [...]
> > @@ -236,14 +241,19 @@ static int histb_pcie_host_enable(struct dw_pcie_rp *pp)
> > goto reg_dis;
> > }
> >
> > - reset_control_assert(hipcie->soft_reset);
> > - reset_control_deassert(hipcie->soft_reset);
> > -
> > - reset_control_assert(hipcie->sys_reset);
> > - reset_control_deassert(hipcie->sys_reset);
> > + ret = reset_control_bulk_assert(PCIE_HISTB_NUM_RESETS,
> > + hipcie->reset);
> > + if (ret) {
> > + dev_err(dev, "Couldn't assert reset %d\n", ret);
> > + goto reg_dis;
> > + }
> >
> > - reset_control_assert(hipcie->bus_reset);
> > - reset_control_deassert(hipcie->bus_reset);
> > + ret = reset_control_bulk_deassert(PCIE_HISTB_NUM_RESETS,
>
> Note that this changes the order of assertion/deassertion, not only
> because resets lines are now switched in bulk, but also because
> reset_control_bulk_deassert() deasserts the reset lines in reverse
> order. So this does
>
> If the three resets are independent and order doesn't matter,
>
I followed the expected reset flow as part of the initialization probe process.
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>
> regards
> Philipp
Thanks
-Anand
© 2016 - 2025 Red Hat, Inc.