| Message ID | 20241112070745.759678-1-jpatel2@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] PCI: armada8k: use reset controller to reset mac | expand |
Observe subject line capitalization convention. On Mon, Nov 11, 2024 at 11:07:45PM -0800, Jenishkumar Maheshbhai Patel wrote: > change mac reset and mac reset bits to reset controller Capitalize sentence. s/mac/MAC/ Explain why we want this. Apparently you're changing from one MAC reset method to another method? Collect these into a series instead of posting individual random patches. > Signed-off-by: Jenishkumar Maheshbhai Patel <jpatel2@marvell.com> > --- > drivers/pci/controller/dwc/pcie-armada8k.c | 30 +++++++--------------- > 1 file changed, 9 insertions(+), 21 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c > index 9a48ef60be51..f9d6907900d1 100644 > --- a/drivers/pci/controller/dwc/pcie-armada8k.c > +++ b/drivers/pci/controller/dwc/pcie-armada8k.c > @@ -21,7 +21,7 @@ > #include <linux/platform_device.h> > #include <linux/resource.h> > #include <linux/of_pci.h> > -#include <linux/mfd/syscon.h> > +#include <linux/reset.h> > #include <linux/regmap.h> > #include <linux/of_gpio.h> > > @@ -35,11 +35,10 @@ struct armada8k_pcie { > struct clk *clk_reg; > struct phy *phy[ARMADA8K_PCIE_MAX_LANES]; > unsigned int phy_count; > - struct regmap *sysctrl_base; > - u32 mac_rest_bitmask; > struct work_struct recover_link_work; > enum of_gpio_flags flags; > struct gpio_desc *reset_gpio; > + struct reset_control *reset; > }; > > #define PCIE_VENDOR_REGS_OFFSET 0x8000 > @@ -257,12 +256,9 @@ static void armada8k_pcie_recover_link(struct work_struct *ws) > msleep(100); > > /* Reset mac */ > - regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG, > - pcie->mac_rest_bitmask, 0, NULL, false, true); > + reset_control_assert(pcie->reset); > udelay(1); > - regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG, > - pcie->mac_rest_bitmask, pcie->mac_rest_bitmask, > - NULL, false, true); > + reset_control_deassert(pcie->reset); > udelay(1); > > ret = dw_pcie_setup_rc(pp); > @@ -331,7 +327,7 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg) > * initiate a link retrain. If link retrains were > * possible, that is. > */ > - if (pcie->sysctrl_base && pcie->mac_rest_bitmask) > + if (pcie->reset) > schedule_work(&pcie->recover_link_work); > > dev_dbg(pci->dev, "%s: link went down\n", __func__); > @@ -440,18 +436,10 @@ static int armada8k_pcie_probe(struct platform_device *pdev) > if (gpio_is_valid(reset_gpio)) > pcie->reset_gpio = gpio_to_desc(reset_gpio); > > - pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, > - "marvell,system-controller"); > - if (IS_ERR(pcie->sysctrl_base)) { > - dev_warn(dev, "failed to find marvell,system-controller\n"); > - pcie->sysctrl_base = 0x0; > - } > - > - ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-bit-mask", > - &pcie->mac_rest_bitmask); > - if (ret < 0) { > - dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret); > - pcie->mac_rest_bitmask = 0x0; > + pcie->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL); > + if (IS_ERR(pcie->reset)) { > + dev_warn(dev, "failed to find mac reset\n"); > + pcie->reset = 0x0; > } > ret = armada8k_pcie_setup_phys(pcie); > if (ret) > -- > 2.25.1 >
> Observe subject line capitalization convention. > > On Mon, Nov 11, 2024 at 11:07:45PM -0800, Jenishkumar Maheshbhai Patel > wrote: > > change mac reset and mac reset bits to reset controller > > Capitalize sentence. > > s/mac/MAC/ > > Explain why we want this. Apparently you're changing from one MAC reset > method to another method? > > Collect these into a series instead of posting individual random patches. > Sorry about the mess. Yes, these patches should be post into a series instead. And this one is actual an improvement to the commit ("PCI: armada8k: Add link-down handle"). It simplified the MAC reset code by reset controller APIs. Eventually, it should be squashed into the previous commit. > > Signed-off-by: Jenishkumar Maheshbhai Patel > > <mailto:jpatel2@marvell.com> > > --- > > drivers/pci/controller/dwc/pcie-armada8k.c | 30 > > +++++++--------------- > > 1 file changed, 9 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c > > b/drivers/pci/controller/dwc/pcie-armada8k.c > > index 9a48ef60be51..f9d6907900d1 100644 > > --- a/drivers/pci/controller/dwc/pcie-armada8k.c > > +++ b/drivers/pci/controller/dwc/pcie-armada8k.c > > @@ -21,7 +21,7 @@ > > #include <linux/platform_device.h> > > #include <linux/resource.h> > > #include <linux/of_pci.h> > > -#include <linux/mfd/syscon.h> > > +#include <linux/reset.h> > > #include <linux/regmap.h> > > #include <linux/of_gpio.h> > > > > @@ -35,11 +35,10 @@ struct armada8k_pcie { > > struct clk *clk_reg; > > struct phy *phy[ARMADA8K_PCIE_MAX_LANES]; > > unsigned int phy_count; > > - struct regmap *sysctrl_base; > > - u32 mac_rest_bitmask; > > struct work_struct recover_link_work; > > enum of_gpio_flags flags; > > struct gpio_desc *reset_gpio; > > + struct reset_control *reset; > > }; > > > > #define PCIE_VENDOR_REGS_OFFSET 0x8000 > > @@ -257,12 +256,9 @@ static void armada8k_pcie_recover_link(struct > work_struct *ws) > > msleep(100); > > > > /* Reset mac */ > > - regmap_update_bits_base(pcie->sysctrl_base, > UNIT_SOFT_RESET_CONFIG_REG, > > - pcie->mac_rest_bitmask, 0, NULL, false, true); > > + reset_control_assert(pcie->reset); > > udelay(1); > > - regmap_update_bits_base(pcie->sysctrl_base, > UNIT_SOFT_RESET_CONFIG_REG, > > - pcie->mac_rest_bitmask, pcie- > >mac_rest_bitmask, > > - NULL, false, true); > > + reset_control_deassert(pcie->reset); > > udelay(1); > > > > ret = dw_pcie_setup_rc(pp); > > @@ -331,7 +327,7 @@ static irqreturn_t armada8k_pcie_irq_handler(int > irq, void *arg) > > * initiate a link retrain. If link retrains were > > * possible, that is. > > */ > > - if (pcie->sysctrl_base && pcie->mac_rest_bitmask) > > + if (pcie->reset) > > schedule_work(&pcie->recover_link_work); > > > > dev_dbg(pci->dev, "%s: link went down\n", __func__); @@ - > 440,18 > > +436,10 @@ static int armada8k_pcie_probe(struct platform_device *pdev) > > if (gpio_is_valid(reset_gpio)) > > pcie->reset_gpio = gpio_to_desc(reset_gpio); > > > > - pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev- > >dev.of_node, > > - "marvell,system- > controller"); > > - if (IS_ERR(pcie->sysctrl_base)) { > > - dev_warn(dev, "failed to find marvell,system-controller\n"); > > - pcie->sysctrl_base = 0x0; > > - } > > - > > - ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset- > bit-mask", > > - &pcie->mac_rest_bitmask); > > - if (ret < 0) { > > - dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret); > > - pcie->mac_rest_bitmask = 0x0; > > + pcie->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL); > > + if (IS_ERR(pcie->reset)) { > > + dev_warn(dev, "failed to find mac reset\n"); > > + pcie->reset = 0x0; > > } > > ret = armada8k_pcie_setup_phys(pcie); > > if (ret) > > -- > > 2.25.1 > >
diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c index 9a48ef60be51..f9d6907900d1 100644 --- a/drivers/pci/controller/dwc/pcie-armada8k.c +++ b/drivers/pci/controller/dwc/pcie-armada8k.c @@ -21,7 +21,7 @@ #include <linux/platform_device.h> #include <linux/resource.h> #include <linux/of_pci.h> -#include <linux/mfd/syscon.h> +#include <linux/reset.h> #include <linux/regmap.h> #include <linux/of_gpio.h> @@ -35,11 +35,10 @@ struct armada8k_pcie { struct clk *clk_reg; struct phy *phy[ARMADA8K_PCIE_MAX_LANES]; unsigned int phy_count; - struct regmap *sysctrl_base; - u32 mac_rest_bitmask; struct work_struct recover_link_work; enum of_gpio_flags flags; struct gpio_desc *reset_gpio; + struct reset_control *reset; }; #define PCIE_VENDOR_REGS_OFFSET 0x8000 @@ -257,12 +256,9 @@ static void armada8k_pcie_recover_link(struct work_struct *ws) msleep(100); /* Reset mac */ - regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG, - pcie->mac_rest_bitmask, 0, NULL, false, true); + reset_control_assert(pcie->reset); udelay(1); - regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG, - pcie->mac_rest_bitmask, pcie->mac_rest_bitmask, - NULL, false, true); + reset_control_deassert(pcie->reset); udelay(1); ret = dw_pcie_setup_rc(pp); @@ -331,7 +327,7 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg) * initiate a link retrain. If link retrains were * possible, that is. */ - if (pcie->sysctrl_base && pcie->mac_rest_bitmask) + if (pcie->reset) schedule_work(&pcie->recover_link_work); dev_dbg(pci->dev, "%s: link went down\n", __func__); @@ -440,18 +436,10 @@ static int armada8k_pcie_probe(struct platform_device *pdev) if (gpio_is_valid(reset_gpio)) pcie->reset_gpio = gpio_to_desc(reset_gpio); - pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, - "marvell,system-controller"); - if (IS_ERR(pcie->sysctrl_base)) { - dev_warn(dev, "failed to find marvell,system-controller\n"); - pcie->sysctrl_base = 0x0; - } - - ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-bit-mask", - &pcie->mac_rest_bitmask); - if (ret < 0) { - dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret); - pcie->mac_rest_bitmask = 0x0; + pcie->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL); + if (IS_ERR(pcie->reset)) { + dev_warn(dev, "failed to find mac reset\n"); + pcie->reset = 0x0; } ret = armada8k_pcie_setup_phys(pcie); if (ret)
change mac reset and mac reset bits to reset controller Signed-off-by: Jenishkumar Maheshbhai Patel <jpatel2@marvell.com> --- drivers/pci/controller/dwc/pcie-armada8k.c | 30 +++++++--------------- 1 file changed, 9 insertions(+), 21 deletions(-)