| Message ID | 20200803061511.29555-5-crystal.guo@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | introduce TI reset controller for MT8192 SoC | expand |
On Mon, 2020-08-03 at 14:15 +0800, Crystal Guo wrote: > Add ti_syscon_reset() to integrate assert and deassert together. > If some modules need do serialized assert and deassert operations > to reset itself, reset_control_reset can be called for convenience. > > Change-Id: I9046992b115a46f3594de57fa89c6a2de9957d49 Please drop "Change-Id" tags. > --- > drivers/reset/reset-ti-syscon.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > index a2635c21db7f..1c74bcb9a6c3 100644 > --- a/drivers/reset/reset-ti-syscon.c > +++ b/drivers/reset/reset-ti-syscon.c > @@ -56,6 +56,7 @@ struct ti_syscon_reset_data { > struct regmap *regmap; > struct ti_syscon_reset_control *controls; > unsigned int nr_controls; > + bool assert_deassert_together; > }; > > #define to_ti_syscon_reset_data(rcdev) \ > @@ -158,10 +159,24 @@ 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) > +{ > + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > + > + if (data->assert_deassert_together) { > + ti_syscon_reset_assert(rcdev, id); > + return ti_syscon_reset_deassert(rcdev, id); > + } else { > + return -ENOTSUPP; > + } > +} > + > 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) > @@ -204,6 +219,11 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > controls[i].flags = be32_to_cpup(list++); > } > > + if (of_property_read_bool(np, "assert-deassert-together")) > + data->assert_deassert_together = true; > + else > + data->assert_deassert_together = false; > + > data->rcdev.ops = &ti_syscon_reset_ops; > data->rcdev.owner = THIS_MODULE; > data->rcdev.of_node = np; Perhaps please provide the reason why you shall add this new method? Any existed or upcoming users? Thanks, Stanley Chu
On Mon, 2020-08-03 at 14:15 +0800, Crystal Guo wrote: > Add ti_syscon_reset() to integrate assert and deassert together. > If some modules need do serialized assert and deassert operations > to reset itself, reset_control_reset can be called for convenience. > > Change-Id: I9046992b115a46f3594de57fa89c6a2de9957d49 > --- > drivers/reset/reset-ti-syscon.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c > index a2635c21db7f..1c74bcb9a6c3 100644 > --- a/drivers/reset/reset-ti-syscon.c > +++ b/drivers/reset/reset-ti-syscon.c > @@ -56,6 +56,7 @@ struct ti_syscon_reset_data { > struct regmap *regmap; > struct ti_syscon_reset_control *controls; > unsigned int nr_controls; > + bool assert_deassert_together; > }; > > #define to_ti_syscon_reset_data(rcdev) \ > @@ -158,10 +159,24 @@ 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) > +{ > + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); > + > + if (data->assert_deassert_together) { > + ti_syscon_reset_assert(rcdev, id); > + return ti_syscon_reset_deassert(rcdev, id); Even if your hardware engineers guarantee that no delays are necessary between assert and deassert for any peripheral used with your reset controller that may use this function, this is not generic. I wonder if it would be better to define a minimum delay similarly to reset-simple [1]. [1] https://lore.kernel.org/lkml/be2cecb2654e68385561a15df7967c7723d5531d.1590594512.git-series.maxime@cerno.tech/ > + } else { > + return -ENOTSUPP; > + } > +} > + > 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) > @@ -204,6 +219,11 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) > controls[i].flags = be32_to_cpup(list++); > } > > + if (of_property_read_bool(np, "assert-deassert-together")) > + data->assert_deassert_together = true; > + else > + data->assert_deassert_together = false; > + This could just be assigned directly, but as said above, I think it might be better to have a reset-duration-us property or similar. regards Philipp
diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c index a2635c21db7f..1c74bcb9a6c3 100644 --- a/drivers/reset/reset-ti-syscon.c +++ b/drivers/reset/reset-ti-syscon.c @@ -56,6 +56,7 @@ struct ti_syscon_reset_data { struct regmap *regmap; struct ti_syscon_reset_control *controls; unsigned int nr_controls; + bool assert_deassert_together; }; #define to_ti_syscon_reset_data(rcdev) \ @@ -158,10 +159,24 @@ 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) +{ + struct ti_syscon_reset_data *data = to_ti_syscon_reset_data(rcdev); + + if (data->assert_deassert_together) { + ti_syscon_reset_assert(rcdev, id); + return ti_syscon_reset_deassert(rcdev, id); + } else { + return -ENOTSUPP; + } +} + 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) @@ -204,6 +219,11 @@ static int ti_syscon_reset_probe(struct platform_device *pdev) controls[i].flags = be32_to_cpup(list++); } + if (of_property_read_bool(np, "assert-deassert-together")) + data->assert_deassert_together = true; + else + data->assert_deassert_together = false; + data->rcdev.ops = &ti_syscon_reset_ops; data->rcdev.owner = THIS_MODULE; data->rcdev.of_node = np;