| Message ID | 20220111075545.1880-2-a-govindraju@ti.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | J721S2: Add initial support | expand |
On Tue, Jan 11, 2022 at 01:25:26PM +0530, Aswath Govindraju wrote: > From: Nishanth Menon <nm@ti.com> > > If there is an optional boot notification channel that an SoC uses > separate from the rx path, use the same. > > Signed-off-by: Nishanth Menon <nm@ti.com> > --- > .../remoteproc/k3-system-controller.txt | 3 +++ > drivers/remoteproc/k3_system_controller.c | 20 ++++++++++++++++++- > 2 files changed, 22 insertions(+), 1 deletion(-) Binding docs are rst these days, so we should sync with upstream and then this property is already there, right?
Hi Tom, On 13/01/22 7:42 pm, Tom Rini wrote: > On Tue, Jan 11, 2022 at 01:25:26PM +0530, Aswath Govindraju wrote: > >> From: Nishanth Menon <nm@ti.com> >> >> If there is an optional boot notification channel that an SoC uses >> separate from the rx path, use the same. >> >> Signed-off-by: Nishanth Menon <nm@ti.com> >> --- >> .../remoteproc/k3-system-controller.txt | 3 +++ >> drivers/remoteproc/k3_system_controller.c | 20 ++++++++++++++++++- >> 2 files changed, 22 insertions(+), 1 deletion(-) > > Binding docs are rst these days, so we should sync with upstream and > then this property is already there, right? > I will create a followup patch to convert documentation to rst. Also, about the property, mbox-names property is already present but "boot_notify" is a newly added channel and not are required property. So, this was additionally added. Thanks, Aswath
Hi Tom, On 17/01/22 11:01 am, Aswath Govindraju wrote: > Hi Tom, > > On 13/01/22 7:42 pm, Tom Rini wrote: >> On Tue, Jan 11, 2022 at 01:25:26PM +0530, Aswath Govindraju wrote: >> >>> From: Nishanth Menon <nm@ti.com> >>> >>> If there is an optional boot notification channel that an SoC uses >>> separate from the rx path, use the same. >>> >>> Signed-off-by: Nishanth Menon <nm@ti.com> >>> --- >>> .../remoteproc/k3-system-controller.txt | 3 +++ >>> drivers/remoteproc/k3_system_controller.c | 20 ++++++++++++++++++- >>> 2 files changed, 22 insertions(+), 1 deletion(-) >> >> Binding docs are rst these days, so we should sync with upstream and >> then this property is already there, right? >> > > I will create a followup patch to convert documentation to rst. Also, > about the property, mbox-names property is already present but > "boot_notify" is a newly added channel and not are required property. > So, this was additionally added. > One more question regarding documentation, should it be changed to rst or yaml, as this is a device tree binding? > Thanks, > Aswath >
On Mon, Jan 17, 2022 at 12:22:52PM +0530, Aswath Govindraju wrote: > Hi Tom, > > On 17/01/22 11:01 am, Aswath Govindraju wrote: > > Hi Tom, > > > > On 13/01/22 7:42 pm, Tom Rini wrote: > >> On Tue, Jan 11, 2022 at 01:25:26PM +0530, Aswath Govindraju wrote: > >> > >>> From: Nishanth Menon <nm@ti.com> > >>> > >>> If there is an optional boot notification channel that an SoC uses > >>> separate from the rx path, use the same. > >>> > >>> Signed-off-by: Nishanth Menon <nm@ti.com> > >>> --- > >>> .../remoteproc/k3-system-controller.txt | 3 +++ > >>> drivers/remoteproc/k3_system_controller.c | 20 ++++++++++++++++++- > >>> 2 files changed, 22 insertions(+), 1 deletion(-) > >> > >> Binding docs are rst these days, so we should sync with upstream and > >> then this property is already there, right? > >> > > > > I will create a followup patch to convert documentation to rst. Also, > > about the property, mbox-names property is already present but > > "boot_notify" is a newly added channel and not are required property. > > So, this was additionally added. > > > > One more question regarding documentation, should it be changed to rst > or yaml, as this is a device tree binding? I mis-spoke, yeah. It should be yaml and pushed upstream first, then brought back here.
Hi Tomi, On 17/01/22 7:24 pm, Tom Rini wrote: > On Mon, Jan 17, 2022 at 12:22:52PM +0530, Aswath Govindraju wrote: >> Hi Tom, >> >> On 17/01/22 11:01 am, Aswath Govindraju wrote: >>> Hi Tom, >>> >>> On 13/01/22 7:42 pm, Tom Rini wrote: >>>> On Tue, Jan 11, 2022 at 01:25:26PM +0530, Aswath Govindraju wrote: >>>> >>>>> From: Nishanth Menon <nm@ti.com> >>>>> >>>>> If there is an optional boot notification channel that an SoC uses >>>>> separate from the rx path, use the same. >>>>> >>>>> Signed-off-by: Nishanth Menon <nm@ti.com> >>>>> --- >>>>> .../remoteproc/k3-system-controller.txt | 3 +++ >>>>> drivers/remoteproc/k3_system_controller.c | 20 ++++++++++++++++++- >>>>> 2 files changed, 22 insertions(+), 1 deletion(-) >>>> >>>> Binding docs are rst these days, so we should sync with upstream and >>>> then this property is already there, right? >>>> >>> >>> I will create a followup patch to convert documentation to rst. Also, >>> about the property, mbox-names property is already present but >>> "boot_notify" is a newly added channel and not are required property. >>> So, this was additionally added. >>> >> >> One more question regarding documentation, should it be changed to rst >> or yaml, as this is a device tree binding? > > I mis-spoke, yeah. It should be yaml and pushed upstream first, then > brought back here. > I am sorry, I have one more question. This above documentation file is not present in kernel documentation, so I did not understand how can this be pushed there first. Also, as converting to yaml would be a different work. Wouldn't it be better to separate that work from this series? Thanks, Aswath
On Tue, Jan 18, 2022 at 11:26:50AM +0530, Aswath Govindraju wrote: > Hi Tomi, > > On 17/01/22 7:24 pm, Tom Rini wrote: > > On Mon, Jan 17, 2022 at 12:22:52PM +0530, Aswath Govindraju wrote: > >> Hi Tom, > >> > >> On 17/01/22 11:01 am, Aswath Govindraju wrote: > >>> Hi Tom, > >>> > >>> On 13/01/22 7:42 pm, Tom Rini wrote: > >>>> On Tue, Jan 11, 2022 at 01:25:26PM +0530, Aswath Govindraju wrote: > >>>> > >>>>> From: Nishanth Menon <nm@ti.com> > >>>>> > >>>>> If there is an optional boot notification channel that an SoC uses > >>>>> separate from the rx path, use the same. > >>>>> > >>>>> Signed-off-by: Nishanth Menon <nm@ti.com> > >>>>> --- > >>>>> .../remoteproc/k3-system-controller.txt | 3 +++ > >>>>> drivers/remoteproc/k3_system_controller.c | 20 ++++++++++++++++++- > >>>>> 2 files changed, 22 insertions(+), 1 deletion(-) > >>>> > >>>> Binding docs are rst these days, so we should sync with upstream and > >>>> then this property is already there, right? > >>>> > >>> > >>> I will create a followup patch to convert documentation to rst. Also, > >>> about the property, mbox-names property is already present but > >>> "boot_notify" is a newly added channel and not are required property. > >>> So, this was additionally added. > >>> > >> > >> One more question regarding documentation, should it be changed to rst > >> or yaml, as this is a device tree binding? > > > > I mis-spoke, yeah. It should be yaml and pushed upstream first, then > > brought back here. > > > > I am sorry, I have one more question. This above documentation file is > not present in kernel documentation, so I did not understand how can > this be pushed there first. > > Also, as converting to yaml would be a different work. Wouldn't it be > better to separate that work from this series? Sigh, it should have been upstreamed first. So yeah, make the changes you need here now and then please start pushing it upstream, thanks.
On 07:50-20220118, Tom Rini wrote: > On Tue, Jan 18, 2022 at 11:26:50AM +0530, Aswath Govindraju wrote: > > Hi Tomi, > > > > On 17/01/22 7:24 pm, Tom Rini wrote: > > > On Mon, Jan 17, 2022 at 12:22:52PM +0530, Aswath Govindraju wrote: > > >> Hi Tom, > > >> > > >> On 17/01/22 11:01 am, Aswath Govindraju wrote: > > >>> Hi Tom, > > >>> > > >>> On 13/01/22 7:42 pm, Tom Rini wrote: > > >>>> On Tue, Jan 11, 2022 at 01:25:26PM +0530, Aswath Govindraju wrote: > > >>>> > > >>>>> From: Nishanth Menon <nm@ti.com> > > >>>>> > > >>>>> If there is an optional boot notification channel that an SoC uses > > >>>>> separate from the rx path, use the same. > > >>>>> > > >>>>> Signed-off-by: Nishanth Menon <nm@ti.com> > > >>>>> --- > > >>>>> .../remoteproc/k3-system-controller.txt | 3 +++ > > >>>>> drivers/remoteproc/k3_system_controller.c | 20 ++++++++++++++++++- > > >>>>> 2 files changed, 22 insertions(+), 1 deletion(-) > > >>>> > > >>>> Binding docs are rst these days, so we should sync with upstream and > > >>>> then this property is already there, right? > > >>>> > > >>> > > >>> I will create a followup patch to convert documentation to rst. Also, > > >>> about the property, mbox-names property is already present but > > >>> "boot_notify" is a newly added channel and not are required property. > > >>> So, this was additionally added. > > >>> > > >> > > >> One more question regarding documentation, should it be changed to rst > > >> or yaml, as this is a device tree binding? > > > > > > I mis-spoke, yeah. It should be yaml and pushed upstream first, then > > > brought back here. > > > > > > > I am sorry, I have one more question. This above documentation file is > > not present in kernel documentation, so I did not understand how can > > this be pushed there first. > > > > Also, as converting to yaml would be a different work. Wouldn't it be > > better to separate that work from this series? > > Sigh, it should have been upstreamed first. So yeah, make the changes > you need here now and then please start pushing it upstream, thanks. Just catching up, but, this goes back to the same question -> this has no relevance beyond R5. what is our current state of sending non-linux dt pieces to upstream kernel? There wont be a driver for sure, neither will there be a direct user in kernel..
On Tue, Jan 18, 2022 at 07:50:34AM -0600, Nishanth Menon wrote: > On 07:50-20220118, Tom Rini wrote: > > On Tue, Jan 18, 2022 at 11:26:50AM +0530, Aswath Govindraju wrote: > > > Hi Tomi, > > > > > > On 17/01/22 7:24 pm, Tom Rini wrote: > > > > On Mon, Jan 17, 2022 at 12:22:52PM +0530, Aswath Govindraju wrote: > > > >> Hi Tom, > > > >> > > > >> On 17/01/22 11:01 am, Aswath Govindraju wrote: > > > >>> Hi Tom, > > > >>> > > > >>> On 13/01/22 7:42 pm, Tom Rini wrote: > > > >>>> On Tue, Jan 11, 2022 at 01:25:26PM +0530, Aswath Govindraju wrote: > > > >>>> > > > >>>>> From: Nishanth Menon <nm@ti.com> > > > >>>>> > > > >>>>> If there is an optional boot notification channel that an SoC uses > > > >>>>> separate from the rx path, use the same. > > > >>>>> > > > >>>>> Signed-off-by: Nishanth Menon <nm@ti.com> > > > >>>>> --- > > > >>>>> .../remoteproc/k3-system-controller.txt | 3 +++ > > > >>>>> drivers/remoteproc/k3_system_controller.c | 20 ++++++++++++++++++- > > > >>>>> 2 files changed, 22 insertions(+), 1 deletion(-) > > > >>>> > > > >>>> Binding docs are rst these days, so we should sync with upstream and > > > >>>> then this property is already there, right? > > > >>>> > > > >>> > > > >>> I will create a followup patch to convert documentation to rst. Also, > > > >>> about the property, mbox-names property is already present but > > > >>> "boot_notify" is a newly added channel and not are required property. > > > >>> So, this was additionally added. > > > >>> > > > >> > > > >> One more question regarding documentation, should it be changed to rst > > > >> or yaml, as this is a device tree binding? > > > > > > > > I mis-spoke, yeah. It should be yaml and pushed upstream first, then > > > > brought back here. > > > > > > > > > > I am sorry, I have one more question. This above documentation file is > > > not present in kernel documentation, so I did not understand how can > > > this be pushed there first. > > > > > > Also, as converting to yaml would be a different work. Wouldn't it be > > > better to separate that work from this series? > > > > Sigh, it should have been upstreamed first. So yeah, make the changes > > you need here now and then please start pushing it upstream, thanks. > > > Just catching up, but, this goes back to the same question -> this has > no relevance beyond R5. what is our current state of sending non-linux > dt pieces to upstream kernel? There wont be a driver for sure, neither > will there be a direct user in kernel.. Non-linux bindings still go upstream. This is probably also a good reminder go poke Rob about the current U-Boot bindings that're otherwise waiting for merge or further comment. dts files are still a follow-up discussion to have, but bindings I believe has been settled and agreed on.
diff --git a/doc/device-tree-bindings/remoteproc/k3-system-controller.txt b/doc/device-tree-bindings/remoteproc/k3-system-controller.txt index 32f4720b0d17..33dc46812ed4 100644 --- a/doc/device-tree-bindings/remoteproc/k3-system-controller.txt +++ b/doc/device-tree-bindings/remoteproc/k3-system-controller.txt @@ -13,6 +13,9 @@ Required properties: "rx" for Receive channel - mboxes: Corresponding phandles to mailbox channels. +Optional properties: +-------------------- +- mbox-names: "boot_notify" for Optional alternate boot notification channel. Example: -------- diff --git a/drivers/remoteproc/k3_system_controller.c b/drivers/remoteproc/k3_system_controller.c index 89cb90207dcb..e2affe69c678 100644 --- a/drivers/remoteproc/k3_system_controller.c +++ b/drivers/remoteproc/k3_system_controller.c @@ -77,14 +77,18 @@ struct k3_sysctrler_desc { * struct k3_sysctrler_privdata - Structure representing System Controller data. * @chan_tx: Transmit mailbox channel * @chan_rx: Receive mailbox channel + * @chan_boot_notify: Boot notification channel * @desc: SoC description for this instance * @seq_nr: Counter for number of messages sent. + * @has_boot_notify: Has separate boot notification channel */ struct k3_sysctrler_privdata { struct mbox_chan chan_tx; struct mbox_chan chan_rx; + struct mbox_chan chan_boot_notify; struct k3_sysctrler_desc *desc; u32 seq_nr; + bool has_boot_notify; }; static inline @@ -223,7 +227,8 @@ static int k3_sysctrler_start(struct udevice *dev) debug("%s(dev=%p)\n", __func__, dev); /* Receive the boot notification. Note that it is sent only once. */ - ret = mbox_recv(&priv->chan_rx, &msg, priv->desc->max_rx_timeout_us); + ret = mbox_recv(priv->has_boot_notify ? &priv->chan_boot_notify : + &priv->chan_rx, &msg, priv->desc->max_rx_timeout_us); if (ret) { dev_err(dev, "%s: Boot Notification response failed. ret = %d\n", __func__, ret); @@ -272,6 +277,19 @@ static int k3_of_to_priv(struct udevice *dev, return ret; } + /* Some SoCs may have a optional channel for boot notification. */ + priv->has_boot_notify = 1; + ret = mbox_get_by_name(dev, "boot_notify", &priv->chan_boot_notify); + if (ret == -ENODATA) { + dev_dbg(dev, "%s: Acquiring optional Boot_notify failed. ret = %d. Using Rx\n", + __func__, ret); + priv->has_boot_notify = 0; + } else if (ret) { + dev_err(dev, "%s: Acquiring boot_notify channel failed. ret = %d\n", + __func__, ret); + return ret; + } + return 0; }