| Message ID | 1479193794-18350-1-git-send-email-xuejiancheng@hisilicon.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Delegated to: | Stephen Boyd |
| Headers | show |
Hi Jiancheng, Am Dienstag, den 15.11.2016, 15:09 +0800 schrieb Jiancheng Xue: > Add a polarity cell for reset line specifier. If the reset line > is asserted when the register bit is 1, the polarity is > normal. Otherwise, it is inverted. > > Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com> > --- > .../devicetree/bindings/clock/hisi-crg.txt | 11 ++++--- > arch/arm/boot/dts/hi3519.dtsi | 2 +- > drivers/clk/hisilicon/reset.c | 36 ++++++++++++++++------ > 3 files changed, 33 insertions(+), 16 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt > index e3919b6..fcbb4f3 100644 > --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt > +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt > @@ -25,19 +25,20 @@ to specify the clock which they consume. > > All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>. > > -- #reset-cells: should be 2. > +- #reset-cells: should be 3. > > A reset signal can be controlled by writing a bit register in the CRG module. > -The reset specifier consists of two cells. The first cell represents the > +The reset specifier consists of three cells. The first cell represents the > register offset relative to the base address. The second cell represents the > -bit index in the register. > +bit index in the register. The third cell represents the polarity of the reset > +line (0 for normal, 1 for inverted). What is normal and what is inverted? Please specify which is active-high and which is active-low. > > Example: CRG nodes > CRG: clock-reset-controller@12010000 { > compatible = "hisilicon,hi3519-crg"; > reg = <0x12010000 0x10000>; > #clock-cells = <1>; > - #reset-cells = <2>; > + #reset-cells = <3>; > }; > > Example: consumer nodes > @@ -45,5 +46,5 @@ i2c0: i2c@12110000 { > compatible = "hisilicon,hi3519-i2c"; > reg = <0x12110000 0x1000>; > clocks = <&CRG HI3519_I2C0_RST>; > - resets = <&CRG 0xe4 0>; > + resets = <&CRG 0xe4 0 0>; > }; > diff --git a/arch/arm/boot/dts/hi3519.dtsi b/arch/arm/boot/dts/hi3519.dtsi > index 5729ecf..b7cb182 100644 > --- a/arch/arm/boot/dts/hi3519.dtsi > +++ b/arch/arm/boot/dts/hi3519.dtsi > @@ -50,7 +50,7 @@ > crg: clock-reset-controller@12010000 { > compatible = "hisilicon,hi3519-crg"; > #clock-cells = <1>; > - #reset-cells = <2>; > + #reset-cells = <3>; That is a backwards incompatible change. Which I think in this case could be tolerated, because there are no users yet of the reset controller. Or are there any hi3519 based device trees that use the resets out in the wild? If there are, the driver must continue to support old device trees with two reset-cells. Which would not be trivial because currently the core checks in reset_control_get that rcdev->of_n_reset_cells is equal to the #reset-cells value from DT. One possibility to get around changing the binding would be to stuff the polarity bit into low bits of the register address cell. Either way, I'm not very happy with blowing up the complexity of the reset phandles at the reset consumer side. If you do change the binding, is there any way you could change from a register address + bit offset binding to an index based binding with the information about reset bit positions and polarities contained in the driver, or in the crg node, similarly to the ti-syscon-reset bindings? That would also improve consistency with clock bindings, which already use a number as identifier. [...] > @@ -59,14 +65,19 @@ static int hisi_reset_assert(struct reset_controller_dev *rcdev, > unsigned long flags; > u32 offset, reg; > u8 bit; > + bool polarity; > > offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT; > - bit = id & HISI_RESET_BIT_MASK; > + bit = (id & HISI_RESET_BIT_MASK) >> HISI_RESET_BIT_SHIFT; > + polarity = id & HISI_RESET_POLARITY_MASK; > > spin_lock_irqsave(&rstc->lock, flags); > > reg = readl(rstc->membase + offset); > - writel(reg | BIT(bit), rstc->membase + offset); > + if (polarity) > + writel(reg & ~BIT(bit), rstc->membase + offset); > + else > + writel(reg | BIT(bit), rstc->membase + offset); So there is no hardware polarity setting, which means the ti-syscon-reset bindings could fit in this case. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Philipp, On 2016/11/15 18:43, Philipp Zabel wrote: > Hi Jiancheng, > > Am Dienstag, den 15.11.2016, 15:09 +0800 schrieb Jiancheng Xue: >> Add a polarity cell for reset line specifier. If the reset line >> is asserted when the register bit is 1, the polarity is >> normal. Otherwise, it is inverted. >> >> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com> >> --- Thank you very much for replying so soon. Please allow me to decribe the reason why this patch exists first. All bits in the reset controller were designed to be active-high. But in a recent chip only one bit was implemented to be active-low :( >> .../devicetree/bindings/clock/hisi-crg.txt | 11 ++++--- >> arch/arm/boot/dts/hi3519.dtsi | 2 +- >> drivers/clk/hisilicon/reset.c | 36 ++++++++++++++++------ >> 3 files changed, 33 insertions(+), 16 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt >> index e3919b6..fcbb4f3 100644 >> --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt >> +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt >> @@ -25,19 +25,20 @@ to specify the clock which they consume. >> >> All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>. >> >> -- #reset-cells: should be 2. >> +- #reset-cells: should be 3. >> >> A reset signal can be controlled by writing a bit register in the CRG module. >> -The reset specifier consists of two cells. The first cell represents the >> +The reset specifier consists of three cells. The first cell represents the >> register offset relative to the base address. The second cell represents the >> -bit index in the register. >> +bit index in the register. The third cell represents the polarity of the reset >> +line (0 for normal, 1 for inverted). > > What is normal and what is inverted? Please specify which is active-high > and which is active-low. > OK. I'll use active-high and active-low instead. >> >> Example: CRG nodes >> CRG: clock-reset-controller@12010000 { >> compatible = "hisilicon,hi3519-crg"; >> reg = <0x12010000 0x10000>; >> #clock-cells = <1>; >> - #reset-cells = <2>; >> + #reset-cells = <3>; >> }; >> >> Example: consumer nodes >> @@ -45,5 +46,5 @@ i2c0: i2c@12110000 { >> compatible = "hisilicon,hi3519-i2c"; >> reg = <0x12110000 0x1000>; >> clocks = <&CRG HI3519_I2C0_RST>; >> - resets = <&CRG 0xe4 0>; >> + resets = <&CRG 0xe4 0 0>; >> }; >> diff --git a/arch/arm/boot/dts/hi3519.dtsi b/arch/arm/boot/dts/hi3519.dtsi >> index 5729ecf..b7cb182 100644 >> --- a/arch/arm/boot/dts/hi3519.dtsi >> +++ b/arch/arm/boot/dts/hi3519.dtsi >> @@ -50,7 +50,7 @@ >> crg: clock-reset-controller@12010000 { >> compatible = "hisilicon,hi3519-crg"; >> #clock-cells = <1>; >> - #reset-cells = <2>; >> + #reset-cells = <3>; > > That is a backwards incompatible change. Which I think in this case > could be tolerated, because there are no users yet of the reset > controller. Or are there any hi3519 based device trees that use the > resets out in the wild? If there are, the driver must continue to > support old device trees with two reset-cells. Which would not be > trivial because currently the core checks in reset_control_get that > rcdev->of_n_reset_cells is equal to the #reset-cells value from DT. I understand the backwards compatiblity is very important. As it can be basically confirmed that the possibility of using hi3519 based device trees is very low, to keep the code simple, I chose to give up the backwards compatiblity. Maybe it is not very convincing. If you think it's better to keep backwards compatiblity here, I can only change reset-cells to 3 for chipsets except Hi3519. > One possibility to get around changing the binding would be to stuff the > polarity bit into low bits of the register address cell. > It's also a solution. But I feel it's not very clear for reset consumer to composite these information together as a index number. Maybe I'm wrong. > Either way, I'm not very happy with blowing up the complexity of the > reset phandles at the reset consumer side. By complexity, do you mean that the consumer side doesn't need to know the detailed information of the implementation of the reset controller eventhough only in the device tree? > If you do change the binding, is there any way you could change from a > register address + bit offset binding to an index based binding with the > information about reset bit positions and polarities contained in the > driver, or in the crg node, similarly to the ti-syscon-reset bindings? > That would also improve consistency with clock bindings, which already > use a number as identifier. > I agree with that this solution is more modular. But the device node of the reset controller will get more complicated. Actually, the device tree of SoC part is provided by SoC vendor. In my opinion, we need balance between reset provider and consumer :). And I supposed rcdev->of_xlate was designed to treat this case. Am I right? > [...] >> @@ -59,14 +65,19 @@ static int hisi_reset_assert(struct reset_controller_dev *rcdev, >> unsigned long flags; >> u32 offset, reg; >> u8 bit; >> + bool polarity; >> >> offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT; >> - bit = id & HISI_RESET_BIT_MASK; >> + bit = (id & HISI_RESET_BIT_MASK) >> HISI_RESET_BIT_SHIFT; >> + polarity = id & HISI_RESET_POLARITY_MASK; >> >> spin_lock_irqsave(&rstc->lock, flags); >> >> reg = readl(rstc->membase + offset); >> - writel(reg | BIT(bit), rstc->membase + offset); >> + if (polarity) >> + writel(reg & ~BIT(bit), rstc->membase + offset); >> + else >> + writel(reg | BIT(bit), rstc->membase + offset); > > So there is no hardware polarity setting, which means the > ti-syscon-reset bindings could fit in this case. > It seems that ti-syscon-reset doesn't support polarity now. Certainly it can be extended through flags. Thank you again. Best Regards, Jiancheng > . > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Philipp, On 2016/11/16 11:17, Jiancheng Xue wrote: > Hi Philipp, > > On 2016/11/15 18:43, Philipp Zabel wrote: >> Hi Jiancheng, >> >> Am Dienstag, den 15.11.2016, 15:09 +0800 schrieb Jiancheng Xue: >>> Add a polarity cell for reset line specifier. If the reset line >>> is asserted when the register bit is 1, the polarity is >>> normal. Otherwise, it is inverted. >>> >>> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com> >>> --- > Thank you very much for replying so soon. > > Please allow me to decribe the reason why this patch exists first. > All bits in the reset controller were designed to be active-high. > But in a recent chip only one bit was implemented to be active-low :( > >>> .../devicetree/bindings/clock/hisi-crg.txt | 11 ++++--- >>> arch/arm/boot/dts/hi3519.dtsi | 2 +- >>> drivers/clk/hisilicon/reset.c | 36 ++++++++++++++++------ >>> 3 files changed, 33 insertions(+), 16 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt >>> index e3919b6..fcbb4f3 100644 >>> --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt >>> +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt >>> @@ -25,19 +25,20 @@ to specify the clock which they consume. >>> >>> All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>. >>> >>> -- #reset-cells: should be 2. >>> +- #reset-cells: should be 3. >>> >>> A reset signal can be controlled by writing a bit register in the CRG module. >>> -The reset specifier consists of two cells. The first cell represents the >>> +The reset specifier consists of three cells. The first cell represents the >>> register offset relative to the base address. The second cell represents the >>> -bit index in the register. >>> +bit index in the register. The third cell represents the polarity of the reset >>> +line (0 for normal, 1 for inverted). >> #reset-cells: Should be 2 if compatilbe string is "hisilicon,hi3519-crg". Should be 3 otherwise. A reset signal can be controlled by writing a bit register in the CRG module. The reset specifier consists of two or three cells. The first cell represents the register offset relative to the base address. The second cell represents the bit index in the register.The third cell represents the polarity of the reset line (0 for active-high, 1 for active-low). If I change the binding like this, can it be accepted? Regards, Jiancheng >> What is normal and what is inverted? Please specify which is active-high >> and which is active-low. >> > OK. I'll use active-high and active-low instead. > >>> >>> Example: CRG nodes >>> CRG: clock-reset-controller@12010000 { >>> compatible = "hisilicon,hi3519-crg"; >>> reg = <0x12010000 0x10000>; >>> #clock-cells = <1>; >>> - #reset-cells = <2>; >>> + #reset-cells = <3>; >>> }; >>> >>> Example: consumer nodes >>> @@ -45,5 +46,5 @@ i2c0: i2c@12110000 { >>> compatible = "hisilicon,hi3519-i2c"; >>> reg = <0x12110000 0x1000>; >>> clocks = <&CRG HI3519_I2C0_RST>; >>> - resets = <&CRG 0xe4 0>; >>> + resets = <&CRG 0xe4 0 0>; >>> }; >>> diff --git a/arch/arm/boot/dts/hi3519.dtsi b/arch/arm/boot/dts/hi3519.dtsi >>> index 5729ecf..b7cb182 100644 >>> --- a/arch/arm/boot/dts/hi3519.dtsi >>> +++ b/arch/arm/boot/dts/hi3519.dtsi >>> @@ -50,7 +50,7 @@ >>> crg: clock-reset-controller@12010000 { >>> compatible = "hisilicon,hi3519-crg"; >>> #clock-cells = <1>; >>> - #reset-cells = <2>; >>> + #reset-cells = <3>; >> >> That is a backwards incompatible change. Which I think in this case >> could be tolerated, because there are no users yet of the reset >> controller. Or are there any hi3519 based device trees that use the >> resets out in the wild? If there are, the driver must continue to >> support old device trees with two reset-cells. Which would not be >> trivial because currently the core checks in reset_control_get that >> rcdev->of_n_reset_cells is equal to the #reset-cells value from DT. > -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016/11/21 10:58, Jiancheng Xue wrote: > Hi Philipp, > >> On 2016/11/15 18:43, Philipp Zabel wrote: >>> Hi Jiancheng, >>> >>> Am Dienstag, den 15.11.2016, 15:09 +0800 schrieb Jiancheng Xue: >>>> Add a polarity cell for reset line specifier. If the reset line >>>> is asserted when the register bit is 1, the polarity is >>>> normal. Otherwise, it is inverted. >>>> >>>> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com> >>>> --- >> Thank you very much for replying so soon. >> >> Please allow me to decribe the reason why this patch exists first. >> All bits in the reset controller were designed to be active-high. >> But in a recent chip only one bit was implemented to be active-low :( >> >>>> .../devicetree/bindings/clock/hisi-crg.txt | 11 ++++--- >>>> arch/arm/boot/dts/hi3519.dtsi | 2 +- >>>> drivers/clk/hisilicon/reset.c | 36 ++++++++++++++++------ >>>> 3 files changed, 33 insertions(+), 16 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt >>>> index e3919b6..fcbb4f3 100644 >>>> --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt >>>> +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt >>>> @@ -25,19 +25,20 @@ to specify the clock which they consume. >>>> >>>> All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>. >>>> >>>> -- #reset-cells: should be 2. >>>> +- #reset-cells: should be 3. >>>> >>>> A reset signal can be controlled by writing a bit register in the CRG module. >>>> -The reset specifier consists of two cells. The first cell represents the >>>> +The reset specifier consists of three cells. The first cell represents the >>>> register offset relative to the base address. The second cell represents the >>>> -bit index in the register. >>>> +bit index in the register. The third cell represents the polarity of the reset >>>> +line (0 for normal, 1 for inverted). >>> > #reset-cells: Should be 2 if compatilbe string is "hisilicon,hi3519-crg". Should be 3 otherwise. > A reset signal can be controlled by writing a bit register in the CRG module. > The reset specifier consists of two or three cells. The first cell represents the > register offset relative to the base address. The second cell represents the > bit index in the register.The third cell represents the polarity of the reset > line (0 for active-high, 1 for active-low). > > If I change the binding like this, can it be accepted? > Hi Philipp, Could you give me more suggestions about this? If you really don't like changing the reset-cells like this, I can modify the patch according to your suggestions. Thank you. Regards, Jiancheng -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016/11/25 11:45, Jiancheng Xue wrote: > > On 2016/11/21 10:58, Jiancheng Xue wrote: >> Hi Philipp, >> >>> On 2016/11/15 18:43, Philipp Zabel wrote: >>>> Hi Jiancheng, >>>> >>>> Am Dienstag, den 15.11.2016, 15:09 +0800 schrieb Jiancheng Xue: >>>>> Add a polarity cell for reset line specifier. If the reset line >>>>> is asserted when the register bit is 1, the polarity is >>>>> normal. Otherwise, it is inverted. >>>>> >>>>> Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com> >>>>> --- >>> Thank you very much for replying so soon. >>> >>> Please allow me to decribe the reason why this patch exists first. >>> All bits in the reset controller were designed to be active-high. >>> But in a recent chip only one bit was implemented to be active-low :( >>> >>>>> .../devicetree/bindings/clock/hisi-crg.txt | 11 ++++--- >>>>> arch/arm/boot/dts/hi3519.dtsi | 2 +- >>>>> drivers/clk/hisilicon/reset.c | 36 ++++++++++++++++------ >>>>> 3 files changed, 33 insertions(+), 16 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt >>>>> index e3919b6..fcbb4f3 100644 >>>>> --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt >>>>> +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt >>>>> @@ -25,19 +25,20 @@ to specify the clock which they consume. >>>>> >>>>> All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>. >>>>> >>>>> -- #reset-cells: should be 2. >>>>> +- #reset-cells: should be 3. >>>>> >>>>> A reset signal can be controlled by writing a bit register in the CRG module. >>>>> -The reset specifier consists of two cells. The first cell represents the >>>>> +The reset specifier consists of three cells. The first cell represents the >>>>> register offset relative to the base address. The second cell represents the >>>>> -bit index in the register. >>>>> +bit index in the register. The third cell represents the polarity of the reset >>>>> +line (0 for normal, 1 for inverted). >>>> >> #reset-cells: Should be 2 if compatilbe string is "hisilicon,hi3519-crg". Should be 3 otherwise. >> A reset signal can be controlled by writing a bit register in the CRG module. >> The reset specifier consists of two or three cells. The first cell represents the >> register offset relative to the base address. The second cell represents the >> bit index in the register.The third cell represents the polarity of the reset >> line (0 for active-high, 1 for active-low). >> >> If I change the binding like this, can it be accepted? >> > Hi Philipp, > > Could you give me more suggestions about this? If you really don't like changing the > reset-cells like this, I can modify the patch according to your suggestions. > Thank you. > I'll drop this patch and use "ti,syscon-reset" instead to resolve the polarity issue. Thanks. Regards, Jiancheng -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/clock/hisi-crg.txt b/Documentation/devicetree/bindings/clock/hisi-crg.txt index e3919b6..fcbb4f3 100644 --- a/Documentation/devicetree/bindings/clock/hisi-crg.txt +++ b/Documentation/devicetree/bindings/clock/hisi-crg.txt @@ -25,19 +25,20 @@ to specify the clock which they consume. All these identifier could be found in <dt-bindings/clock/hi3519-clock.h>. -- #reset-cells: should be 2. +- #reset-cells: should be 3. A reset signal can be controlled by writing a bit register in the CRG module. -The reset specifier consists of two cells. The first cell represents the +The reset specifier consists of three cells. The first cell represents the register offset relative to the base address. The second cell represents the -bit index in the register. +bit index in the register. The third cell represents the polarity of the reset +line (0 for normal, 1 for inverted). Example: CRG nodes CRG: clock-reset-controller@12010000 { compatible = "hisilicon,hi3519-crg"; reg = <0x12010000 0x10000>; #clock-cells = <1>; - #reset-cells = <2>; + #reset-cells = <3>; }; Example: consumer nodes @@ -45,5 +46,5 @@ i2c0: i2c@12110000 { compatible = "hisilicon,hi3519-i2c"; reg = <0x12110000 0x1000>; clocks = <&CRG HI3519_I2C0_RST>; - resets = <&CRG 0xe4 0>; + resets = <&CRG 0xe4 0 0>; }; diff --git a/arch/arm/boot/dts/hi3519.dtsi b/arch/arm/boot/dts/hi3519.dtsi index 5729ecf..b7cb182 100644 --- a/arch/arm/boot/dts/hi3519.dtsi +++ b/arch/arm/boot/dts/hi3519.dtsi @@ -50,7 +50,7 @@ crg: clock-reset-controller@12010000 { compatible = "hisilicon,hi3519-crg"; #clock-cells = <1>; - #reset-cells = <2>; + #reset-cells = <3>; reg = <0x12010000 0x10000>; }; diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c index 2a5015c..c0ab0b6 100644 --- a/drivers/clk/hisilicon/reset.c +++ b/drivers/clk/hisilicon/reset.c @@ -17,6 +17,7 @@ * along with this program. If not, see <http://www.gnu.org/licenses/>. */ +#include <linux/bitops.h> #include <linux/io.h> #include <linux/of_address.h> #include <linux/platform_device.h> @@ -25,9 +26,11 @@ #include <linux/spinlock.h> #include "reset.h" -#define HISI_RESET_BIT_MASK 0x1f -#define HISI_RESET_OFFSET_SHIFT 8 -#define HISI_RESET_OFFSET_MASK 0xffff00 +#define HISI_RESET_POLARITY_MASK BIT(0) +#define HISI_RESET_BIT_SHIFT 1 +#define HISI_RESET_BIT_MASK GENMASK(6, 1) +#define HISI_RESET_OFFSET_SHIFT 8 +#define HISI_RESET_OFFSET_MASK GENMASK(23, 8) struct hisi_reset_controller { spinlock_t lock; @@ -44,12 +47,15 @@ static int hisi_reset_of_xlate(struct reset_controller_dev *rcdev, { u32 offset; u8 bit; + bool polarity; offset = (reset_spec->args[0] << HISI_RESET_OFFSET_SHIFT) & HISI_RESET_OFFSET_MASK; - bit = reset_spec->args[1] & HISI_RESET_BIT_MASK; + bit = (reset_spec->args[1] << HISI_RESET_BIT_SHIFT) + & HISI_RESET_BIT_MASK; + polarity = reset_spec->args[2] & HISI_RESET_POLARITY_MASK; - return (offset | bit); + return (offset | bit | polarity); } static int hisi_reset_assert(struct reset_controller_dev *rcdev, @@ -59,14 +65,19 @@ static int hisi_reset_assert(struct reset_controller_dev *rcdev, unsigned long flags; u32 offset, reg; u8 bit; + bool polarity; offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT; - bit = id & HISI_RESET_BIT_MASK; + bit = (id & HISI_RESET_BIT_MASK) >> HISI_RESET_BIT_SHIFT; + polarity = id & HISI_RESET_POLARITY_MASK; spin_lock_irqsave(&rstc->lock, flags); reg = readl(rstc->membase + offset); - writel(reg | BIT(bit), rstc->membase + offset); + if (polarity) + writel(reg & ~BIT(bit), rstc->membase + offset); + else + writel(reg | BIT(bit), rstc->membase + offset); spin_unlock_irqrestore(&rstc->lock, flags); @@ -80,14 +91,19 @@ static int hisi_reset_deassert(struct reset_controller_dev *rcdev, unsigned long flags; u32 offset, reg; u8 bit; + bool polarity; offset = (id & HISI_RESET_OFFSET_MASK) >> HISI_RESET_OFFSET_SHIFT; - bit = id & HISI_RESET_BIT_MASK; + bit = (id & HISI_RESET_BIT_MASK) >> HISI_RESET_BIT_SHIFT; + polarity = id & HISI_RESET_POLARITY_MASK; spin_lock_irqsave(&rstc->lock, flags); reg = readl(rstc->membase + offset); - writel(reg & ~BIT(bit), rstc->membase + offset); + if (polarity) + writel(reg | BIT(bit), rstc->membase + offset); + else + writel(reg & ~BIT(bit), rstc->membase + offset); spin_unlock_irqrestore(&rstc->lock, flags); @@ -118,7 +134,7 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev) rstc->rcdev.owner = THIS_MODULE; rstc->rcdev.ops = &hisi_reset_ops; rstc->rcdev.of_node = pdev->dev.of_node; - rstc->rcdev.of_reset_n_cells = 2; + rstc->rcdev.of_reset_n_cells = 3; rstc->rcdev.of_xlate = hisi_reset_of_xlate; reset_controller_register(&rstc->rcdev);
Add a polarity cell for reset line specifier. If the reset line is asserted when the register bit is 1, the polarity is normal. Otherwise, it is inverted. Signed-off-by: Jiancheng Xue <xuejiancheng@hisilicon.com> --- .../devicetree/bindings/clock/hisi-crg.txt | 11 ++++--- arch/arm/boot/dts/hi3519.dtsi | 2 +- drivers/clk/hisilicon/reset.c | 36 ++++++++++++++++------ 3 files changed, 33 insertions(+), 16 deletions(-)