* [PATCH 0/2] adjust the reset assert and deassert interface @ 2020-07-29 7:39 Crystal Guo 2020-07-29 7:39 ` [PATCH 1/2] reset-controller: ti: " Crystal Guo 2020-07-29 7:39 ` [PATCH 2/2] arm64: dts: mt8192: add infracfg_rst node Crystal Guo 0 siblings, 2 replies; 8+ messages in thread From: Crystal Guo @ 2020-07-29 7:39 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, seiya.wang 1. adjust the reset assert and deassert interface to make it more compatible. 2. add infracfg_rst device node. Crystal Guo (2): reset-controller: ti: adjust the reset assert and deassert interface arm64: dts: mt8192: add infracfg_rst node arch/arm64/boot/dts/mediatek/mt8192.dtsi | 10 +++++++++- drivers/reset/reset-ti-syscon.c | 13 +++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] reset-controller: ti: adjust the reset assert and deassert interface 2020-07-29 7:39 [PATCH 0/2] adjust the reset assert and deassert interface Crystal Guo @ 2020-07-29 7:39 ` Crystal Guo 2020-07-29 7:48 ` Matthias Brugger 2020-07-29 7:39 ` [PATCH 2/2] arm64: dts: mt8192: add infracfg_rst node Crystal Guo 1 sibling, 1 reply; 8+ messages in thread From: Crystal Guo @ 2020-07-29 7:39 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, seiya.wang, Crystal Guo Add ti_syscon_reset() to integrate assert and deassert together, and change return value of the reset assert and deassert interface from regmap_update_bits to regmap_write_bits. when clear bit is already 1, regmap_update_bits can not write 1 to it again. Some IC has the feature that, when set bit is 1, the clear bit change to 1 together. It will truly clear bit to 0 by write 1 to the clear bit Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- drivers/reset/reset-ti-syscon.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c index a2635c2..5a8ec8f 100644 --- a/drivers/reset/reset-ti-syscon.c +++ b/drivers/reset/reset-ti-syscon.c @@ -89,7 +89,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, mask = BIT(control->assert_bit); value = (control->flags & ASSERT_SET) ? mask : 0x0; - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); } /** @@ -120,7 +120,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, mask = BIT(control->deassert_bit); value = (control->flags & DEASSERT_SET) ? mask : 0x0; - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); } /** @@ -158,10 +158,19 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, !(control->flags & STATUS_SET); } +static int ti_syscon_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + ti_syscon_reset_assert(rcdev, id); + + return ti_syscon_reset_deassert(rcdev, id); +} + static const struct reset_control_ops ti_syscon_reset_ops = { .assert = ti_syscon_reset_assert, .deassert = ti_syscon_reset_deassert, .status = ti_syscon_reset_status, + .reset = ti_syscon_reset, }; static int ti_syscon_reset_probe(struct platform_device *pdev) -- 1.8.1.1.dirty ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] reset-controller: ti: adjust the reset assert and deassert interface 2020-07-29 7:39 ` [PATCH 1/2] reset-controller: ti: " Crystal Guo @ 2020-07-29 7:48 ` Matthias Brugger 2020-07-29 8:02 ` Philipp Zabel 0 siblings, 1 reply; 8+ messages in thread From: Matthias Brugger @ 2020-07-29 7:48 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, seiya.wang On 29/07/2020 09:39, Crystal Guo wrote: > Add ti_syscon_reset() to integrate assert and deassert together, > and change return value of the reset assert and deassert interface > from regmap_update_bits to regmap_write_bits. > > when clear bit is already 1, regmap_update_bits can not write 1 to it again. > Some IC has the feature that, when set bit is 1, the clear bit change > to 1 together. It will truly clear bit to 0 by write 1 to the clear bit > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > drivers/reset/reset-ti-syscon.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > index a2635c2..5a8ec8f 100644 > --- a/drivers/reset/reset-ti-syscon.c > +++ b/drivers/reset/reset-ti-syscon.c > @@ -89,7 +89,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > mask = BIT(control->assert_bit); > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); Nack, this will break the driver for the other devices. The kernel has to work not just for your SoC but for all devices of all architectures. You can't just hack something up, that will work on your specific SoC. Regards, Matthias > } > > /** > @@ -120,7 +120,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > mask = BIT(control->deassert_bit); > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > } > > /** > @@ -158,10 +158,19 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > !(control->flags & STATUS_SET); > } > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + ti_syscon_reset_assert(rcdev, id); > + > + return ti_syscon_reset_deassert(rcdev, id); > +} > + > static const struct reset_control_ops ti_syscon_reset_ops = { > .assert = ti_syscon_reset_assert, > .deassert = ti_syscon_reset_deassert, > .status = ti_syscon_reset_status, > + .reset = ti_syscon_reset, > }; > > static int ti_syscon_reset_probe(struct platform_device *pdev) > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] reset-controller: ti: adjust the reset assert and deassert interface 2020-07-29 7:48 ` Matthias Brugger @ 2020-07-29 8:02 ` Philipp Zabel 2020-07-30 6:27 ` Crystal Guo 0 siblings, 1 reply; 8+ messages in thread From: Philipp Zabel @ 2020-07-29 8:02 UTC (permalink / raw) To: Matthias Brugger, Crystal Guo, robh+dt Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, seiya.wang Hi Crystal, Matthias, On Wed, 2020-07-29 at 09:48 +0200, Matthias Brugger wrote: > > On 29/07/2020 09:39, Crystal Guo wrote: > > Add ti_syscon_reset() to integrate assert and deassert together, > > and change return value of the reset assert and deassert interface > > from regmap_update_bits to regmap_write_bits. > > > > when clear bit is already 1, regmap_update_bits can not write 1 to it again. > > Some IC has the feature that, when set bit is 1, the clear bit change > > to 1 together. It will truly clear bit to 0 by write 1 to the clear bit > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > drivers/reset/reset-ti-syscon.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > > index a2635c2..5a8ec8f 100644 > > --- a/drivers/reset/reset-ti-syscon.c > > +++ b/drivers/reset/reset-ti-syscon.c > > @@ -89,7 +89,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > > mask = BIT(control->assert_bit); > > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > > Nack, this will break the driver for the other devices. I don't think this will break the driver for existing hardware. regmap_write_bits() is the same as regmap_update_bits(), it just forces the write in case the read already happens to return the correct value. Of course it would be good to check that this actually works. > The kernel has to work not just for your SoC but for all devices of all > architectures. You can't just hack something up, that will work on your specific > SoC. > > Regards, > Matthias > > > } > > > > /** > > @@ -120,7 +120,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > > mask = BIT(control->deassert_bit); > > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > > } > > > > /** > > @@ -158,10 +158,19 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > > !(control->flags & STATUS_SET); > > } > > > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > > + unsigned long id) > > +{ > > + ti_syscon_reset_assert(rcdev, id); > > + > > + return ti_syscon_reset_deassert(rcdev, id); > > +} > > + I'm unsure about this one, though. This is an incompatible change. At the very least this would have to be optional depending on compatible. regards Philipp ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] reset-controller: ti: adjust the reset assert and deassert interface 2020-07-29 8:02 ` Philipp Zabel @ 2020-07-30 6:27 ` Crystal Guo 0 siblings, 0 replies; 8+ messages in thread From: Crystal Guo @ 2020-07-30 6:27 UTC (permalink / raw) To: Philipp Zabel Cc: Matthias Brugger, robh+dt, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君), yingjoe.chen, fan.chen, stanley.chu On Wed, 2020-07-29 at 16:02 +0800, Philipp Zabel wrote: > Hi Crystal, Matthias, > > On Wed, 2020-07-29 at 09:48 +0200, Matthias Brugger wrote: > > > > On 29/07/2020 09:39, Crystal Guo wrote: > > > Add ti_syscon_reset() to integrate assert and deassert together, > > > and change return value of the reset assert and deassert interface > > > from regmap_update_bits to regmap_write_bits. > > > > > > when clear bit is already 1, regmap_update_bits can not write 1 to it again. > > > Some IC has the feature that, when set bit is 1, the clear bit change > > > to 1 together. It will truly clear bit to 0 by write 1 to the clear bit > > > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > > --- > > > drivers/reset/reset-ti-syscon.c | 13 +++++++++++-- > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > > > index a2635c2..5a8ec8f 100644 > > > --- a/drivers/reset/reset-ti-syscon.c > > > +++ b/drivers/reset/reset-ti-syscon.c > > > @@ -89,7 +89,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev, > > > mask = BIT(control->assert_bit); > > > value = (control->flags & ASSERT_SET) ? mask : 0x0; > > > > > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value); > > > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value); > > > > Nack, this will break the driver for the other devices. > > I don't think this will break the driver for existing hardware. > regmap_write_bits() is the same as regmap_update_bits(), it just forces > the write in case the read already happens to return the correct value. > Of course it would be good to check that this actually works. Yes, regmap_write_bits() is the same as regmap_update_bits(), it would not affect existed users. Or should I use a property to separate regmap_write_bits() and regmap_update_bits() ? > > > The kernel has to work not just for your SoC but for all devices of all > > architectures. You can't just hack something up, that will work on your specific > > SoC. > > > > Regards, > > Matthias > > This TI driver was intend to be a generic reset controller (https://lore.kernel.org/patchwork/cover/683585/), so this patch may not just work on a specific SoC. Thanks, Crystal > > > } > > > > > > /** > > > @@ -120,7 +120,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev, > > > mask = BIT(control->deassert_bit); > > > value = (control->flags & DEASSERT_SET) ? mask : 0x0; > > > > > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value); > > > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value); > > > } > > > > > > /** > > > @@ -158,10 +158,19 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev, > > > !(control->flags & STATUS_SET); > > > } > > > > > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev, > > > + unsigned long id) > > > +{ > > > + ti_syscon_reset_assert(rcdev, id); > > > + > > > + return ti_syscon_reset_deassert(rcdev, id); > > > +} > > > + > > I'm unsure about this one, though. This is an incompatible change. At > the very least this would have to be optional depending on compatible. > > regards > Philipp I will add a property to make this change be optional, thanks for you advice. Thanks, Crystal ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] arm64: dts: mt8192: add infracfg_rst node 2020-07-29 7:39 [PATCH 0/2] adjust the reset assert and deassert interface Crystal Guo 2020-07-29 7:39 ` [PATCH 1/2] reset-controller: ti: " Crystal Guo @ 2020-07-29 7:39 ` Crystal Guo 2020-07-29 7:45 ` Matthias Brugger 1 sibling, 1 reply; 8+ messages in thread From: Crystal Guo @ 2020-07-29 7:39 UTC (permalink / raw) To: p.zabel, robh+dt, matthias.bgg Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, seiya.wang, Crystal Guo add infracfg_rst node which is for MT8192 platform Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- arch/arm64/boot/dts/mediatek/mt8192.dtsi | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi index b16dbbd..adc6239 100644 --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi @@ -217,9 +217,17 @@ }; infracfg: infracfg@10001000 { - compatible = "mediatek,mt8192-infracfg", "syscon"; + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; reg = <0 0x10001000 0 0x1000>; #clock-cells = <1>; + + infracfg_rst: reset-controller { + compatible = "ti,syscon-reset"; + #reset-cells = <1>; + ti,reset-bits = < + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ + >; + }; }; pericfg: pericfg@10003000 { -- 1.8.1.1.dirty ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8192: add infracfg_rst node 2020-07-29 7:39 ` [PATCH 2/2] arm64: dts: mt8192: add infracfg_rst node Crystal Guo @ 2020-07-29 7:45 ` Matthias Brugger 2020-07-29 8:18 ` Crystal Guo 0 siblings, 1 reply; 8+ messages in thread From: Matthias Brugger @ 2020-07-29 7:45 UTC (permalink / raw) To: Crystal Guo, p.zabel, robh+dt Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, seiya.wang On 29/07/2020 09:39, Crystal Guo wrote: > add infracfg_rst node which is for MT8192 platform > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > index b16dbbd..adc6239 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > @@ -217,9 +217,17 @@ > }; > > infracfg: infracfg@10001000 { > - compatible = "mediatek,mt8192-infracfg", "syscon"; > + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; > reg = <0 0x10001000 0 0x1000>; > #clock-cells = <1>; > + > + infracfg_rst: reset-controller { > + compatible = "ti,syscon-reset"; > + #reset-cells = <1>; > + ti,reset-bits = < > + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ You have Texas Instruments hardware inside infracfg? Are you sure? > + >; > + }; > }; > > pericfg: pericfg@10003000 { > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] arm64: dts: mt8192: add infracfg_rst node 2020-07-29 7:45 ` Matthias Brugger @ 2020-07-29 8:18 ` Crystal Guo 0 siblings, 0 replies; 8+ messages in thread From: Crystal Guo @ 2020-07-29 8:18 UTC (permalink / raw) To: Matthias Brugger Cc: p.zabel, robh+dt, srv_heupstream, linux-mediatek, linux-arm-kernel, linux-kernel, devicetree, Seiya Wang (王迺君) On Wed, 2020-07-29 at 15:45 +0800, Matthias Brugger wrote: > > On 29/07/2020 09:39, Crystal Guo wrote: > > add infracfg_rst node which is for MT8192 platform > > > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > > --- > > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > index b16dbbd..adc6239 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > @@ -217,9 +217,17 @@ > > }; > > > > infracfg: infracfg@10001000 { > > - compatible = "mediatek,mt8192-infracfg", "syscon"; > > + compatible = "mediatek,mt8192-infracfg", "syscon", "simple-mfd"; > > reg = <0 0x10001000 0 0x1000>; > > #clock-cells = <1>; > > + > > + infracfg_rst: reset-controller { > > + compatible = "ti,syscon-reset"; > > + #reset-cells = <1>; > > + ti,reset-bits = < > > + 0x140 15 0x144 15 0 0 (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: pcie */ > > You have Texas Instruments hardware inside infracfg? Are you sure? > TI reset-controller driver is a common driver, MTK SOC has the similar control flow, thus can reuse it. > > + >; > > + }; > > }; > > > > pericfg: pericfg@10003000 { > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-30 6:27 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-29 7:39 [PATCH 0/2] adjust the reset assert and deassert interface Crystal Guo 2020-07-29 7:39 ` [PATCH 1/2] reset-controller: ti: " Crystal Guo 2020-07-29 7:48 ` Matthias Brugger 2020-07-29 8:02 ` Philipp Zabel 2020-07-30 6:27 ` Crystal Guo 2020-07-29 7:39 ` [PATCH 2/2] arm64: dts: mt8192: add infracfg_rst node Crystal Guo 2020-07-29 7:45 ` Matthias Brugger 2020-07-29 8:18 ` Crystal Guo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).