Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 07/07/17 14:12, Jassi Brar wrote: > On Fri, Jul 7, 2017 at 5:02 PM, Sudeep Hollawrote: >> >> >> On 06/07/17 19:37, Jassi Brar wrote: >>> On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Holla wrote: On 06/07/17 15:37, Jassi Brar wrote: > On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: >>> > I see no reason why you must have SCPI and SCMI both running. > We can still have 2 different protocols using same MHU channel with different doorbells, what's wrong with that ? >>> Only for SCMI and SCPI - both written by same person to achieve the >>> same thing - I think it should not be necessary. >>> >> >> Really ? you decide based on that and repeatedly ignore what ARM MHU >> specification offers. Wow, I am surprised. >> >>> But yes, SCMI running alongside some complementary protocol (that >>> implements what SCMI doesn't provide) is a very solid usecase. And >>> that is the reason you need a platform specific 'transport layer'. >>> >> >> The whole SCMI exercise is to reduce such platform specific code. >> It's made to work with ACPI using doorbells via PCC channels. Now, you >> say for DT we need per platform shim. Mailbox is my transport, why do I >> need anything more as I don't care what is sent in mailbox controller as >> long as the signal is sent to the other side. >> > And even then there is a solution - a shim arbitrator. Other > platforms, those share a channel, do that. No big deal. > Example please? Please remember these protocols are generic and we can't add any platform specific code into them. >>> You do need to have platform specific glue as I said. >>> >> >> Yes, please propose along bindings to do that as you are objecting the >> one in $subject. I have asked you many times the same thing. >> > "platform specific transport layer" => no generic bindings for the > controller. > It's not platform specific. It's in MHU specification and please stop making it platform specific just because your platform didn't use it. STOP this nonsense argument and read the MHU specification. I am tired of mentioning that again and again. > The bindings, that already exist for mailbox controllers plus what > properties the SCMI need, are to be used by a thin platform specific > driver to provide an interface to the generic SCMI implementation to > send the "doorbell". > It's non-sense to provide such layer for each and every controller and platform. MHU supports doorbells and I need to add that support. You still fail to make any technical issues adding such feature. What is the problem you see adding that feature. Please list them. > For example, look at how the common DWC3 driver is used by platforms > that have different h/w integrations. >$ grep synopsys,dwc3 -rw -B15 arch/arm/boot/dts/ > > That is, SCMI node as a child of platform specific SCMI node. Or maybe > even SCMI as a library kinda thing. > 1. It's not topic of this patch 2. SCMI is discoverable and we want to have as minimum binding as possible and now you are asking to add unnecessary layers of binding which I don't agree with and I am fine if you propose and convince DT maintainers. > BTW, I hope you realise that we need a 'transport layer' which will > be the platform specific glue between mailbox controller specifics and > the generic SCMI code. Why ? >>> Because you should not restrict the usage of SCMI to only platforms >>> with mailbox controller that does not require any information to >>> trigger a signal on the other side -- what you call "doorbell". >>> >> >> Indeed, it's restricted, read the specification >> > Its a shame to hear that. > Sorry, but I am not author of the specification. Also just FYI it was reviewed and accepted by multiple vendors. >>> Why shouldn't Rockchip be able to implement SCMI? Just because it uses >>> different mechanism to signal the other end? >>> >> >> They can, all we need is just a mailbox channel, we don't care wants >> sent in the controller as along as signal is sent. The shared memory has >> to be as per the specification. I still don't know what I am missing to >> convey you. >> > > In scmi_do_xfer() you pass pointer to 'struct scmi_xfer' as 'data'. That is > > mbox_send_message(struct mbox_chan *, struct scmi_xfer*); > > A controller driver defines is own format for the data it needs to > send a message. > Exactly the point and that's why I didn't bother much to drag you into SCMI. The main reason why I am adding doorbel to MHU is not just SCMI. It's to support multiple protocols implemented using different bit of each channel as a doorbell. Please don't link that with SCMI. You are the one who wanted to look at the code. Hence I pointed to SCMI, I am *not* saying SCMI cares what MHU driver send. But as another requirement, I have 4 different channel with 2 different protocol. It may also
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 07/07/17 14:12, Jassi Brar wrote: > On Fri, Jul 7, 2017 at 5:02 PM, Sudeep Holla wrote: >> >> >> On 06/07/17 19:37, Jassi Brar wrote: >>> On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Holla wrote: On 06/07/17 15:37, Jassi Brar wrote: > On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: >>> > I see no reason why you must have SCPI and SCMI both running. > We can still have 2 different protocols using same MHU channel with different doorbells, what's wrong with that ? >>> Only for SCMI and SCPI - both written by same person to achieve the >>> same thing - I think it should not be necessary. >>> >> >> Really ? you decide based on that and repeatedly ignore what ARM MHU >> specification offers. Wow, I am surprised. >> >>> But yes, SCMI running alongside some complementary protocol (that >>> implements what SCMI doesn't provide) is a very solid usecase. And >>> that is the reason you need a platform specific 'transport layer'. >>> >> >> The whole SCMI exercise is to reduce such platform specific code. >> It's made to work with ACPI using doorbells via PCC channels. Now, you >> say for DT we need per platform shim. Mailbox is my transport, why do I >> need anything more as I don't care what is sent in mailbox controller as >> long as the signal is sent to the other side. >> > And even then there is a solution - a shim arbitrator. Other > platforms, those share a channel, do that. No big deal. > Example please? Please remember these protocols are generic and we can't add any platform specific code into them. >>> You do need to have platform specific glue as I said. >>> >> >> Yes, please propose along bindings to do that as you are objecting the >> one in $subject. I have asked you many times the same thing. >> > "platform specific transport layer" => no generic bindings for the > controller. > It's not platform specific. It's in MHU specification and please stop making it platform specific just because your platform didn't use it. STOP this nonsense argument and read the MHU specification. I am tired of mentioning that again and again. > The bindings, that already exist for mailbox controllers plus what > properties the SCMI need, are to be used by a thin platform specific > driver to provide an interface to the generic SCMI implementation to > send the "doorbell". > It's non-sense to provide such layer for each and every controller and platform. MHU supports doorbells and I need to add that support. You still fail to make any technical issues adding such feature. What is the problem you see adding that feature. Please list them. > For example, look at how the common DWC3 driver is used by platforms > that have different h/w integrations. >$ grep synopsys,dwc3 -rw -B15 arch/arm/boot/dts/ > > That is, SCMI node as a child of platform specific SCMI node. Or maybe > even SCMI as a library kinda thing. > 1. It's not topic of this patch 2. SCMI is discoverable and we want to have as minimum binding as possible and now you are asking to add unnecessary layers of binding which I don't agree with and I am fine if you propose and convince DT maintainers. > BTW, I hope you realise that we need a 'transport layer' which will > be the platform specific glue between mailbox controller specifics and > the generic SCMI code. Why ? >>> Because you should not restrict the usage of SCMI to only platforms >>> with mailbox controller that does not require any information to >>> trigger a signal on the other side -- what you call "doorbell". >>> >> >> Indeed, it's restricted, read the specification >> > Its a shame to hear that. > Sorry, but I am not author of the specification. Also just FYI it was reviewed and accepted by multiple vendors. >>> Why shouldn't Rockchip be able to implement SCMI? Just because it uses >>> different mechanism to signal the other end? >>> >> >> They can, all we need is just a mailbox channel, we don't care wants >> sent in the controller as along as signal is sent. The shared memory has >> to be as per the specification. I still don't know what I am missing to >> convey you. >> > > In scmi_do_xfer() you pass pointer to 'struct scmi_xfer' as 'data'. That is > > mbox_send_message(struct mbox_chan *, struct scmi_xfer*); > > A controller driver defines is own format for the data it needs to > send a message. > Exactly the point and that's why I didn't bother much to drag you into SCMI. The main reason why I am adding doorbel to MHU is not just SCMI. It's to support multiple protocols implemented using different bit of each channel as a doorbell. Please don't link that with SCMI. You are the one who wanted to look at the code. Hence I pointed to SCMI, I am *not* saying SCMI cares what MHU driver send. But as another requirement, I have 4 different channel with 2 different protocol. It may also extend to 6 different channels where we want to reserve 2 for DVFS
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Fri, Jul 7, 2017 at 5:02 PM, Sudeep Hollawrote: > > > On 06/07/17 19:37, Jassi Brar wrote: >> On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Holla wrote: >>> >>> On 06/07/17 15:37, Jassi Brar wrote: On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: >>> >> I see no reason why you must have SCPI and SCMI both running. >>> >>> We can still have 2 different protocols using same MHU channel with >>> different doorbells, what's wrong with that ? >>> >> Only for SCMI and SCPI - both written by same person to achieve the >> same thing - I think it should not be necessary. >> > > Really ? you decide based on that and repeatedly ignore what ARM MHU > specification offers. Wow, I am surprised. > >> But yes, SCMI running alongside some complementary protocol (that >> implements what SCMI doesn't provide) is a very solid usecase. And >> that is the reason you need a platform specific 'transport layer'. >> > > The whole SCMI exercise is to reduce such platform specific code. > It's made to work with ACPI using doorbells via PCC channels. Now, you > say for DT we need per platform shim. Mailbox is my transport, why do I > need anything more as I don't care what is sent in mailbox controller as > long as the signal is sent to the other side. > And even then there is a solution - a shim arbitrator. Other platforms, those share a channel, do that. No big deal. >>> >>> Example please? Please remember these protocols are generic and we >>> can't add any platform specific code into them. >>> >> You do need to have platform specific glue as I said. >> > > Yes, please propose along bindings to do that as you are objecting the > one in $subject. I have asked you many times the same thing. > "platform specific transport layer" => no generic bindings for the controller. The bindings, that already exist for mailbox controllers plus what properties the SCMI need, are to be used by a thin platform specific driver to provide an interface to the generic SCMI implementation to send the "doorbell". For example, look at how the common DWC3 driver is used by platforms that have different h/w integrations. $ grep synopsys,dwc3 -rw -B15 arch/arm/boot/dts/ That is, SCMI node as a child of platform specific SCMI node. Or maybe even SCMI as a library kinda thing. I can't explain it any better. BTW, I hope you realise that we need a 'transport layer' which will be the platform specific glue between mailbox controller specifics and the generic SCMI code. >>> >>> Why ? >>> >> Because you should not restrict the usage of SCMI to only platforms >> with mailbox controller that does not require any information to >> trigger a signal on the other side -- what you call "doorbell". >> > > Indeed, it's restricted, read the specification > Its a shame to hear that. >> Why shouldn't Rockchip be able to implement SCMI? Just because it uses >> different mechanism to signal the other end? >> > > They can, all we need is just a mailbox channel, we don't care wants > sent in the controller as along as signal is sent. The shared memory has > to be as per the specification. I still don't know what I am missing to > convey you. > In scmi_do_xfer() you pass pointer to 'struct scmi_xfer' as 'data'. That is mbox_send_message(struct mbox_chan *, struct scmi_xfer*); A controller driver defines is own format for the data it needs to send a message. For example, Rockchip driver expects rockchip_mbox_send_data(struct mbox_chan *, struct rockchip_mbox_msg *) Fow the huck will it work ?! SCMI is purely a s/w protocol which can work over any physical link. But it wouldn't because you have funny notions of a "doorbell".
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Fri, Jul 7, 2017 at 5:02 PM, Sudeep Holla wrote: > > > On 06/07/17 19:37, Jassi Brar wrote: >> On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Holla wrote: >>> >>> On 06/07/17 15:37, Jassi Brar wrote: On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: >>> >> I see no reason why you must have SCPI and SCMI both running. >>> >>> We can still have 2 different protocols using same MHU channel with >>> different doorbells, what's wrong with that ? >>> >> Only for SCMI and SCPI - both written by same person to achieve the >> same thing - I think it should not be necessary. >> > > Really ? you decide based on that and repeatedly ignore what ARM MHU > specification offers. Wow, I am surprised. > >> But yes, SCMI running alongside some complementary protocol (that >> implements what SCMI doesn't provide) is a very solid usecase. And >> that is the reason you need a platform specific 'transport layer'. >> > > The whole SCMI exercise is to reduce such platform specific code. > It's made to work with ACPI using doorbells via PCC channels. Now, you > say for DT we need per platform shim. Mailbox is my transport, why do I > need anything more as I don't care what is sent in mailbox controller as > long as the signal is sent to the other side. > And even then there is a solution - a shim arbitrator. Other platforms, those share a channel, do that. No big deal. >>> >>> Example please? Please remember these protocols are generic and we >>> can't add any platform specific code into them. >>> >> You do need to have platform specific glue as I said. >> > > Yes, please propose along bindings to do that as you are objecting the > one in $subject. I have asked you many times the same thing. > "platform specific transport layer" => no generic bindings for the controller. The bindings, that already exist for mailbox controllers plus what properties the SCMI need, are to be used by a thin platform specific driver to provide an interface to the generic SCMI implementation to send the "doorbell". For example, look at how the common DWC3 driver is used by platforms that have different h/w integrations. $ grep synopsys,dwc3 -rw -B15 arch/arm/boot/dts/ That is, SCMI node as a child of platform specific SCMI node. Or maybe even SCMI as a library kinda thing. I can't explain it any better. BTW, I hope you realise that we need a 'transport layer' which will be the platform specific glue between mailbox controller specifics and the generic SCMI code. >>> >>> Why ? >>> >> Because you should not restrict the usage of SCMI to only platforms >> with mailbox controller that does not require any information to >> trigger a signal on the other side -- what you call "doorbell". >> > > Indeed, it's restricted, read the specification > Its a shame to hear that. >> Why shouldn't Rockchip be able to implement SCMI? Just because it uses >> different mechanism to signal the other end? >> > > They can, all we need is just a mailbox channel, we don't care wants > sent in the controller as along as signal is sent. The shared memory has > to be as per the specification. I still don't know what I am missing to > convey you. > In scmi_do_xfer() you pass pointer to 'struct scmi_xfer' as 'data'. That is mbox_send_message(struct mbox_chan *, struct scmi_xfer*); A controller driver defines is own format for the data it needs to send a message. For example, Rockchip driver expects rockchip_mbox_send_data(struct mbox_chan *, struct rockchip_mbox_msg *) Fow the huck will it work ?! SCMI is purely a s/w protocol which can work over any physical link. But it wouldn't because you have funny notions of a "doorbell".
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 06/07/17 19:37, Jassi Brar wrote: > On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Hollawrote: >> >> On 06/07/17 15:37, Jassi Brar wrote: >>> On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: >> > >>> I see no reason why you must have SCPI and SCMI both running. >>> >> >> We can still have 2 different protocols using same MHU channel with >> different doorbells, what's wrong with that ? >> > Only for SCMI and SCPI - both written by same person to achieve the > same thing - I think it should not be necessary. > Really ? you decide based on that and repeatedly ignore what ARM MHU specification offers. Wow, I am surprised. > But yes, SCMI running alongside some complementary protocol (that > implements what SCMI doesn't provide) is a very solid usecase. And > that is the reason you need a platform specific 'transport layer'. > The whole SCMI exercise is to reduce such platform specific code. It's made to work with ACPI using doorbells via PCC channels. Now, you say for DT we need per platform shim. Mailbox is my transport, why do I need anything more as I don't care what is sent in mailbox controller as long as the signal is sent to the other side. >>> And even then there is a solution - a shim arbitrator. Other >>> platforms, those share a channel, do that. No big deal. >>> >> >> Example please? Please remember these protocols are generic and we >> can't add any platform specific code into them. >> > You do need to have platform specific glue as I said. > Yes, please propose along bindings to do that as you are objecting the one in $subject. I have asked you many times the same thing. >>> BTW, I hope you realise that we need a 'transport layer' which will >>> be the platform specific glue between mailbox controller specifics and >>> the generic SCMI code. >> >> Why ? >> > Because you should not restrict the usage of SCMI to only platforms > with mailbox controller that does not require any information to > trigger a signal on the other side -- what you call "doorbell". > Indeed, it's restricted, read the specification. We have all the information in the channel shared memory and we just need doorbell in the transport. It's clear in the specification. You seem to not read any of the specification and continue to ignore. > Why shouldn't Rockchip be able to implement SCMI? Just because it uses > different mechanism to signal the other end? > They can, all we need is just a mailbox channel, we don't care wants sent in the controller as along as signal is sent. The shared memory has to be as per the specification. I still don't know what I am missing to convey you. > You are actually putting in effort to restrict the usage of SCMI. Its > like you buy a usb device and the first thing you do is solder it to > the plug. > I don't think you have read the SCMI specification, sorry. > >> Clearly you have not made a since technical argument so far as why >> MHU doorbell is not correct way even when MHU specification is clearly >> allows it. I have given example of ST mailbox which has this doorbell >> kind of support. >> >>> I see your confusion in the form of some issues in the SCMI >>> implementation, please CC me on the next revision. >>> >> >> Care to elaborate on what's my confusion or at-least what you think so ? >> > See me last point. > I think you are just fixiated on one usage of ARM MHU and so hard to convince otherwise.. >> Also if you have concern on implementation, ok we can discuss further. >> But can you make it clear as what your objections are for the doorbell >> MHU binding. How will I get the bit assigned for different protocols >> which are platform specific ? I still need some binding , right ? >> > Sorry, No. I'll try to get lkml fwd me the patchset so I can explain > there further. > Sure, thanks. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 06/07/17 19:37, Jassi Brar wrote: > On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Holla wrote: >> >> On 06/07/17 15:37, Jassi Brar wrote: >>> On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: >> > >>> I see no reason why you must have SCPI and SCMI both running. >>> >> >> We can still have 2 different protocols using same MHU channel with >> different doorbells, what's wrong with that ? >> > Only for SCMI and SCPI - both written by same person to achieve the > same thing - I think it should not be necessary. > Really ? you decide based on that and repeatedly ignore what ARM MHU specification offers. Wow, I am surprised. > But yes, SCMI running alongside some complementary protocol (that > implements what SCMI doesn't provide) is a very solid usecase. And > that is the reason you need a platform specific 'transport layer'. > The whole SCMI exercise is to reduce such platform specific code. It's made to work with ACPI using doorbells via PCC channels. Now, you say for DT we need per platform shim. Mailbox is my transport, why do I need anything more as I don't care what is sent in mailbox controller as long as the signal is sent to the other side. >>> And even then there is a solution - a shim arbitrator. Other >>> platforms, those share a channel, do that. No big deal. >>> >> >> Example please? Please remember these protocols are generic and we >> can't add any platform specific code into them. >> > You do need to have platform specific glue as I said. > Yes, please propose along bindings to do that as you are objecting the one in $subject. I have asked you many times the same thing. >>> BTW, I hope you realise that we need a 'transport layer' which will >>> be the platform specific glue between mailbox controller specifics and >>> the generic SCMI code. >> >> Why ? >> > Because you should not restrict the usage of SCMI to only platforms > with mailbox controller that does not require any information to > trigger a signal on the other side -- what you call "doorbell". > Indeed, it's restricted, read the specification. We have all the information in the channel shared memory and we just need doorbell in the transport. It's clear in the specification. You seem to not read any of the specification and continue to ignore. > Why shouldn't Rockchip be able to implement SCMI? Just because it uses > different mechanism to signal the other end? > They can, all we need is just a mailbox channel, we don't care wants sent in the controller as along as signal is sent. The shared memory has to be as per the specification. I still don't know what I am missing to convey you. > You are actually putting in effort to restrict the usage of SCMI. Its > like you buy a usb device and the first thing you do is solder it to > the plug. > I don't think you have read the SCMI specification, sorry. > >> Clearly you have not made a since technical argument so far as why >> MHU doorbell is not correct way even when MHU specification is clearly >> allows it. I have given example of ST mailbox which has this doorbell >> kind of support. >> >>> I see your confusion in the form of some issues in the SCMI >>> implementation, please CC me on the next revision. >>> >> >> Care to elaborate on what's my confusion or at-least what you think so ? >> > See me last point. > I think you are just fixiated on one usage of ARM MHU and so hard to convince otherwise.. >> Also if you have concern on implementation, ok we can discuss further. >> But can you make it clear as what your objections are for the doorbell >> MHU binding. How will I get the bit assigned for different protocols >> which are platform specific ? I still need some binding , right ? >> > Sorry, No. I'll try to get lkml fwd me the patchset so I can explain > there further. > Sure, thanks. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Hollawrote: > > On 06/07/17 15:37, Jassi Brar wrote: >> On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: > >> I see no reason why you must have SCPI and SCMI both running. >> > > We can still have 2 different protocols using same MHU channel with > different doorbells, what's wrong with that ? > Only for SCMI and SCPI - both written by same person to achieve the same thing - I think it should not be necessary. But yes, SCMI running alongside some complementary protocol (that implements what SCMI doesn't provide) is a very solid usecase. And that is the reason you need a platform specific 'transport layer'. >> And even then there is a solution - a shim arbitrator. Other >> platforms, those share a channel, do that. No big deal. >> > > Example please? Please remember these protocols are generic and we > can't add any platform specific code into them. > You do need to have platform specific glue as I said. >> BTW, I hope you realise that we need a 'transport layer' which will >> be the platform specific glue between mailbox controller specifics and >> the generic SCMI code. > > Why ? > Because you should not restrict the usage of SCMI to only platforms with mailbox controller that does not require any information to trigger a signal on the other side -- what you call "doorbell". Why shouldn't Rockchip be able to implement SCMI? Just because it uses different mechanism to signal the other end? You are actually putting in effort to restrict the usage of SCMI. Its like you buy a usb device and the first thing you do is solder it to the plug. > Clearly you have not made a since technical argument so far as why > MHU doorbell is not correct way even when MHU specification is clearly > allows it. I have given example of ST mailbox which has this doorbell > kind of support. > >> I see your confusion in the form of some issues in the SCMI >> implementation, please CC me on the next revision. >> > > Care to elaborate on what's my confusion or at-least what you think so ? > See me last point. > Also if you have concern on implementation, ok we can discuss further. > But can you make it clear as what your objections are for the doorbell > MHU binding. How will I get the bit assigned for different protocols > which are platform specific ? I still need some binding , right ? > Sorry, No. I'll try to get lkml fwd me the patchset so I can explain there further.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Thu, Jul 6, 2017 at 10:14 PM, Sudeep Holla wrote: > > On 06/07/17 15:37, Jassi Brar wrote: >> On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: > >> I see no reason why you must have SCPI and SCMI both running. >> > > We can still have 2 different protocols using same MHU channel with > different doorbells, what's wrong with that ? > Only for SCMI and SCPI - both written by same person to achieve the same thing - I think it should not be necessary. But yes, SCMI running alongside some complementary protocol (that implements what SCMI doesn't provide) is a very solid usecase. And that is the reason you need a platform specific 'transport layer'. >> And even then there is a solution - a shim arbitrator. Other >> platforms, those share a channel, do that. No big deal. >> > > Example please? Please remember these protocols are generic and we > can't add any platform specific code into them. > You do need to have platform specific glue as I said. >> BTW, I hope you realise that we need a 'transport layer' which will >> be the platform specific glue between mailbox controller specifics and >> the generic SCMI code. > > Why ? > Because you should not restrict the usage of SCMI to only platforms with mailbox controller that does not require any information to trigger a signal on the other side -- what you call "doorbell". Why shouldn't Rockchip be able to implement SCMI? Just because it uses different mechanism to signal the other end? You are actually putting in effort to restrict the usage of SCMI. Its like you buy a usb device and the first thing you do is solder it to the plug. > Clearly you have not made a since technical argument so far as why > MHU doorbell is not correct way even when MHU specification is clearly > allows it. I have given example of ST mailbox which has this doorbell > kind of support. > >> I see your confusion in the form of some issues in the SCMI >> implementation, please CC me on the next revision. >> > > Care to elaborate on what's my confusion or at-least what you think so ? > See me last point. > Also if you have concern on implementation, ok we can discuss further. > But can you make it clear as what your objections are for the doorbell > MHU binding. How will I get the bit assigned for different protocols > which are platform specific ? I still need some binding , right ? > Sorry, No. I'll try to get lkml fwd me the patchset so I can explain there further.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 06/07/17 15:37, Jassi Brar wrote: > On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Hollawrote: [...] >> >> I said it *may not be used*, currently it is used. >> > SCPI provides more than what SCMI currently does - dvfs, clock, sensor. Not sure what you mean by that, but that's not true. > I see no reason why you must have SCPI and SCMI both running. > We can still have 2 different protocols using same MHU channel with different doorbells, what's wrong with that ? > And even then there is a solution - a shim arbitrator. Other > platforms, those share a channel, do that. No big deal. > Example please ? Please remember these protocols are generic and we can't add any platform specific code into them. > BTW, I hope you realise that we need a 'transport layer' which will > be the platform specific glue between mailbox controller specifics and > the generic SCMI code. Why ? Clearly you have not made a since technical argument so far as why MHU doorbell is not correct way even when MHU specification is clearly allows it. I have given example of ST mailbox which has this doorbell kind of support. > I see your confusion in the form of some issues in the SCMI > implementation, please CC me on the next revision. > Care to elaborate on what's my confusion or at-least what you think so ? Also if you have concern on implementation, ok we can discuss further. But can you make it clear as what your objections are for the doorbell MHU binding. How will I get the bit assigned for different protocols which are platform specific ? I still need some binding , right ? -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 06/07/17 15:37, Jassi Brar wrote: > On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: [...] >> >> I said it *may not be used*, currently it is used. >> > SCPI provides more than what SCMI currently does - dvfs, clock, sensor. Not sure what you mean by that, but that's not true. > I see no reason why you must have SCPI and SCMI both running. > We can still have 2 different protocols using same MHU channel with different doorbells, what's wrong with that ? > And even then there is a solution - a shim arbitrator. Other > platforms, those share a channel, do that. No big deal. > Example please ? Please remember these protocols are generic and we can't add any platform specific code into them. > BTW, I hope you realise that we need a 'transport layer' which will > be the platform specific glue between mailbox controller specifics and > the generic SCMI code. Why ? Clearly you have not made a since technical argument so far as why MHU doorbell is not correct way even when MHU specification is clearly allows it. I have given example of ST mailbox which has this doorbell kind of support. > I see your confusion in the form of some issues in the SCMI > implementation, please CC me on the next revision. > Care to elaborate on what's my confusion or at-least what you think so ? Also if you have concern on implementation, ok we can discuss further. But can you make it clear as what your objections are for the doorbell MHU binding. How will I get the bit assigned for different protocols which are platform specific ? I still need some binding , right ? -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Hollawrote: > > > On 06/07/17 10:27, Jassi Brar wrote: >> On Thu, Jul 6, 2017 at 2:48 PM, Sudeep Holla wrote: >>> Hi Jassi, >>> >>> On 06/07/17 07:28, Jassi Brar wrote: On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Holla wrote: > > I have posted the SCMI patches now[1], > I wish I was CC'ed on that. Now LKML seems too busy to forward it. >>> >>> Yes, my mistake, I should have cc-ed you. >>> > please let me know how to get > both SCPI and SCMI working together with different doorbell bits on the > same channel. > You say in the cover letter : "Let me begin admitting that we are introducing yet another protocol to achieve same things as many existing protocols like ARM SCPI, TI SCI, QCOM RPM, Nvidia Tegra BPMP, and so on" So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is to be used for future platforms. If SCPI and SCMI achieve the same, why have them both active simultaneously? >>> >>> Yes it may not be used, but the firmware might support both for backward >>> compatibility. E.g. on Juno, we still may continue supporting SCPI while >>> we transition to SCMI. So both old and new DTs must work. >>> >> Sure, but still there is no reason to have both SCMI and SCPI active >> during _runtime_. >> Either SCMI or SCPI should be populated by DT, not both. >> Assuming there really is some sane excuse :- >>> >>> Yes as I mentioned above. >>> >> If you specify only one of SCPI/SCMI, you wouldn't need the shim arbitrator. >> > > I said it *may not be used*, currently it is used. > SCPI provides more than what SCMI currently does - dvfs, clock, sensor. I see no reason why you must have SCPI and SCMI both running. And even then there is a solution - a shim arbitrator. Other platforms, those share a channel, do that. No big deal. BTW, I hope you realise that we need a 'transport layer' which will be the platform specific glue between mailbox controller specifics and the generic SCMI code. I see your confusion in the form of some issues in the SCMI implementation, please CC me on the next revision. Thanks.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Thu, Jul 6, 2017 at 3:03 PM, Sudeep Holla wrote: > > > On 06/07/17 10:27, Jassi Brar wrote: >> On Thu, Jul 6, 2017 at 2:48 PM, Sudeep Holla wrote: >>> Hi Jassi, >>> >>> On 06/07/17 07:28, Jassi Brar wrote: On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Holla wrote: > > I have posted the SCMI patches now[1], > I wish I was CC'ed on that. Now LKML seems too busy to forward it. >>> >>> Yes, my mistake, I should have cc-ed you. >>> > please let me know how to get > both SCPI and SCMI working together with different doorbell bits on the > same channel. > You say in the cover letter : "Let me begin admitting that we are introducing yet another protocol to achieve same things as many existing protocols like ARM SCPI, TI SCI, QCOM RPM, Nvidia Tegra BPMP, and so on" So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is to be used for future platforms. If SCPI and SCMI achieve the same, why have them both active simultaneously? >>> >>> Yes it may not be used, but the firmware might support both for backward >>> compatibility. E.g. on Juno, we still may continue supporting SCPI while >>> we transition to SCMI. So both old and new DTs must work. >>> >> Sure, but still there is no reason to have both SCMI and SCPI active >> during _runtime_. >> Either SCMI or SCPI should be populated by DT, not both. >> Assuming there really is some sane excuse :- >>> >>> Yes as I mentioned above. >>> >> If you specify only one of SCPI/SCMI, you wouldn't need the shim arbitrator. >> > > I said it *may not be used*, currently it is used. > SCPI provides more than what SCMI currently does - dvfs, clock, sensor. I see no reason why you must have SCPI and SCMI both running. And even then there is a solution - a shim arbitrator. Other platforms, those share a channel, do that. No big deal. BTW, I hope you realise that we need a 'transport layer' which will be the platform specific glue between mailbox controller specifics and the generic SCMI code. I see your confusion in the form of some issues in the SCMI implementation, please CC me on the next revision. Thanks.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 06/07/17 10:27, Jassi Brar wrote: > On Thu, Jul 6, 2017 at 2:48 PM, Sudeep Hollawrote: >> Hi Jassi, >> >> On 06/07/17 07:28, Jassi Brar wrote: >>> On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Holla wrote: >>> I have posted the SCMI patches now[1], >>> I wish I was CC'ed on that. Now LKML seems too busy to forward it. >>> >> >> Yes, my mistake, I should have cc-ed you. >> please let me know how to get both SCPI and SCMI working together with different doorbell bits on the same channel. >>> You say in the cover letter : >>> "Let me begin admitting that we are introducing yet another protocol to >>> achieve same things as many existing protocols like ARM SCPI, TI SCI, >>> QCOM RPM, Nvidia Tegra BPMP, and so on" >>> >>> So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is >>> to be used for future platforms. >>> If SCPI and SCMI achieve the same, why have them both active simultaneously? >>> >> >> Yes it may not be used, but the firmware might support both for backward >> compatibility. E.g. on Juno, we still may continue supporting SCPI while >> we transition to SCMI. So both old and new DTs must work. >> > Sure, but still there is no reason to have both SCMI and SCPI active > during _runtime_. > Either SCMI or SCPI should be populated by DT, not both. > >>> Assuming there really is some sane excuse :- >> >> Yes as I mentioned above. >> > If you specify only one of SCPI/SCMI, you wouldn't need the shim arbitrator. > I said it *may not be used*, currently it is used. Also can you please answer other questions I had on mailbox API and not breaking SCPI exiting users ? I need your suggestion to proceed on that. Similar to SCPI, SCMI will be used by other platforms which must have doorbell mailbox mechanism. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 06/07/17 10:27, Jassi Brar wrote: > On Thu, Jul 6, 2017 at 2:48 PM, Sudeep Holla wrote: >> Hi Jassi, >> >> On 06/07/17 07:28, Jassi Brar wrote: >>> On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Holla wrote: >>> I have posted the SCMI patches now[1], >>> I wish I was CC'ed on that. Now LKML seems too busy to forward it. >>> >> >> Yes, my mistake, I should have cc-ed you. >> please let me know how to get both SCPI and SCMI working together with different doorbell bits on the same channel. >>> You say in the cover letter : >>> "Let me begin admitting that we are introducing yet another protocol to >>> achieve same things as many existing protocols like ARM SCPI, TI SCI, >>> QCOM RPM, Nvidia Tegra BPMP, and so on" >>> >>> So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is >>> to be used for future platforms. >>> If SCPI and SCMI achieve the same, why have them both active simultaneously? >>> >> >> Yes it may not be used, but the firmware might support both for backward >> compatibility. E.g. on Juno, we still may continue supporting SCPI while >> we transition to SCMI. So both old and new DTs must work. >> > Sure, but still there is no reason to have both SCMI and SCPI active > during _runtime_. > Either SCMI or SCPI should be populated by DT, not both. > >>> Assuming there really is some sane excuse :- >> >> Yes as I mentioned above. >> > If you specify only one of SCPI/SCMI, you wouldn't need the shim arbitrator. > I said it *may not be used*, currently it is used. Also can you please answer other questions I had on mailbox API and not breaking SCPI exiting users ? I need your suggestion to proceed on that. Similar to SCPI, SCMI will be used by other platforms which must have doorbell mailbox mechanism. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Thu, Jul 6, 2017 at 2:48 PM, Sudeep Hollawrote: > Hi Jassi, > > On 06/07/17 07:28, Jassi Brar wrote: >> On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Holla wrote: >> >>> >>> I have posted the SCMI patches now[1], >>> >> I wish I was CC'ed on that. Now LKML seems too busy to forward it. >> > > Yes, my mistake, I should have cc-ed you. > >>> please let me know how to get >>> both SCPI and SCMI working together with different doorbell bits on the >>> same channel. >>> >> You say in the cover letter : >> "Let me begin admitting that we are introducing yet another protocol to >> achieve same things as many existing protocols like ARM SCPI, TI SCI, >> QCOM RPM, Nvidia Tegra BPMP, and so on" >> >> So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is >> to be used for future platforms. >> If SCPI and SCMI achieve the same, why have them both active simultaneously? >> > > Yes it may not be used, but the firmware might support both for backward > compatibility. E.g. on Juno, we still may continue supporting SCPI while > we transition to SCMI. So both old and new DTs must work. > Sure, but still there is no reason to have both SCMI and SCPI active during _runtime_. Either SCMI or SCPI should be populated by DT, not both. >> Assuming there really is some sane excuse :- > > Yes as I mentioned above. > If you specify only one of SCPI/SCMI, you wouldn't need the shim arbitrator.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Thu, Jul 6, 2017 at 2:48 PM, Sudeep Holla wrote: > Hi Jassi, > > On 06/07/17 07:28, Jassi Brar wrote: >> On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Holla wrote: >> >>> >>> I have posted the SCMI patches now[1], >>> >> I wish I was CC'ed on that. Now LKML seems too busy to forward it. >> > > Yes, my mistake, I should have cc-ed you. > >>> please let me know how to get >>> both SCPI and SCMI working together with different doorbell bits on the >>> same channel. >>> >> You say in the cover letter : >> "Let me begin admitting that we are introducing yet another protocol to >> achieve same things as many existing protocols like ARM SCPI, TI SCI, >> QCOM RPM, Nvidia Tegra BPMP, and so on" >> >> So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is >> to be used for future platforms. >> If SCPI and SCMI achieve the same, why have them both active simultaneously? >> > > Yes it may not be used, but the firmware might support both for backward > compatibility. E.g. on Juno, we still may continue supporting SCPI while > we transition to SCMI. So both old and new DTs must work. > Sure, but still there is no reason to have both SCMI and SCPI active during _runtime_. Either SCMI or SCPI should be populated by DT, not both. >> Assuming there really is some sane excuse :- > > Yes as I mentioned above. > If you specify only one of SCPI/SCMI, you wouldn't need the shim arbitrator.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
Hi Jassi, On 06/07/17 07:28, Jassi Brar wrote: > On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Hollawrote: > >> >> I have posted the SCMI patches now[1], >> > I wish I was CC'ed on that. Now LKML seems too busy to forward it. > Yes, my mistake, I should have cc-ed you. >> please let me know how to get >> both SCPI and SCMI working together with different doorbell bits on the >> same channel. >> > You say in the cover letter : > "Let me begin admitting that we are introducing yet another protocol to > achieve same things as many existing protocols like ARM SCPI, TI SCI, > QCOM RPM, Nvidia Tegra BPMP, and so on" > > So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is > to be used for future platforms. > If SCPI and SCMI achieve the same, why have them both active simultaneously? > Yes it may not be used, but the firmware might support both for backward compatibility. E.g. on Juno, we still may continue supporting SCPI while we transition to SCMI. So both old and new DTs must work. > Assuming there really is some sane excuse :- Yes as I mentioned above. > SCPI and SCMI are two separate client working over an MHU channel. > So, as is the case with users relying on a common resource, you need a > shim arbitrator that serialises access to the resource.> Yes, that's what I have done with this series of patches retaining the backward compatibility. I have made it generic to ARM MHU instead of specifically addressing couple of protocols on a particular platform. If I try to come up with an additional shim driver, I bet it will become much ugly, bigger and platform specific. The specification of MHU mentions on how to use it as doorbell bit and I am doing that to provide cleaner and generic solution which I you seem to disagree with. So please suggest/provide alternative solution. A shim layer may get complicated as both SCMI and SCPI will call standard mailbox APIs. Do you mean I need to replace those calls with shim layer API ? If I do that it may break on other platforms which are using SCPI. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
Hi Jassi, On 06/07/17 07:28, Jassi Brar wrote: > On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Holla wrote: > >> >> I have posted the SCMI patches now[1], >> > I wish I was CC'ed on that. Now LKML seems too busy to forward it. > Yes, my mistake, I should have cc-ed you. >> please let me know how to get >> both SCPI and SCMI working together with different doorbell bits on the >> same channel. >> > You say in the cover letter : > "Let me begin admitting that we are introducing yet another protocol to > achieve same things as many existing protocols like ARM SCPI, TI SCI, > QCOM RPM, Nvidia Tegra BPMP, and so on" > > So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is > to be used for future platforms. > If SCPI and SCMI achieve the same, why have them both active simultaneously? > Yes it may not be used, but the firmware might support both for backward compatibility. E.g. on Juno, we still may continue supporting SCPI while we transition to SCMI. So both old and new DTs must work. > Assuming there really is some sane excuse :- Yes as I mentioned above. > SCPI and SCMI are two separate client working over an MHU channel. > So, as is the case with users relying on a common resource, you need a > shim arbitrator that serialises access to the resource.> Yes, that's what I have done with this series of patches retaining the backward compatibility. I have made it generic to ARM MHU instead of specifically addressing couple of protocols on a particular platform. If I try to come up with an additional shim driver, I bet it will become much ugly, bigger and platform specific. The specification of MHU mentions on how to use it as doorbell bit and I am doing that to provide cleaner and generic solution which I you seem to disagree with. So please suggest/provide alternative solution. A shim layer may get complicated as both SCMI and SCPI will call standard mailbox APIs. Do you mean I need to replace those calls with shim layer API ? If I do that it may break on other platforms which are using SCPI. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Hollawrote: > > I have posted the SCMI patches now[1], > I wish I was CC'ed on that. Now LKML seems too busy to forward it. > please let me know how to get > both SCPI and SCMI working together with different doorbell bits on the > same channel. > You say in the cover letter : "Let me begin admitting that we are introducing yet another protocol to achieve same things as many existing protocols like ARM SCPI, TI SCI, QCOM RPM, Nvidia Tegra BPMP, and so on" So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is to be used for future platforms. If SCPI and SCMI achieve the same, why have them both active simultaneously? Assuming there really is some sane excuse :- SCPI and SCMI are two separate client working over an MHU channel. So, as is the case with users relying on a common resource, you need a shim arbitrator that serialises access to the resource.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Wed, Jul 5, 2017 at 11:32 PM, Sudeep Holla wrote: > > I have posted the SCMI patches now[1], > I wish I was CC'ed on that. Now LKML seems too busy to forward it. > please let me know how to get > both SCPI and SCMI working together with different doorbell bits on the > same channel. > You say in the cover letter : "Let me begin admitting that we are introducing yet another protocol to achieve same things as many existing protocols like ARM SCPI, TI SCI, QCOM RPM, Nvidia Tegra BPMP, and so on" So SCMI is supposed to replace SCPI, SCI, RPM and BPMP or SCMI is to be used for future platforms. If SCPI and SCMI achieve the same, why have them both active simultaneously? Assuming there really is some sane excuse :- SCPI and SCMI are two separate client working over an MHU channel. So, as is the case with users relying on a common resource, you need a shim arbitrator that serialises access to the resource.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 02/06/17 10:32, Sudeep Holla wrote: > > > On 02/06/17 06:45, Jassi Brar wrote: >> Hi Rob, >> >> On Wed, May 31, 2017 at 10:38 PM, Rob Herringwrote: >>> On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 >> -- >> 1 file changed, 43 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> index 4971f03f0b33..bd9a3a267caf 100644 >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having >> read the data. >> The last channel is specified to be a 'Secure' resource, hence can't be >> used by Linux running NS. >> >> +The MHU drives the interrupt signal using a 32-bit register, with all >> +32-bits logically ORed together. It provides a set of registers to >> +enable software to set, clear and check the status of each of the bits >> +of this register independently. The use of 32 bits per interrupt line >> +enables software to provide more information about the source of the >> +interrupt. For example, each bit of the register can be associated with >> +a type of event that can contribute to raising the interrupt. Each of >> +the 32-bits can be used as "doorbell" to alert the remote processor. >> + >> Mailbox Device Node: >> >> >> Required properties: >> >> -- compatible: Shall be "arm,mhu" & "arm,primecell" >> +- compatible: Shall be "arm,primecell" and one of the below: >> + "arm,mhu" - if the controller doesn't support >> + doorbell model >> + "arm,mhu-doorbell" - if the controller supports >> + doorbell model >> - reg: Contains the mailbox register address range (base >> address and length) >> -- #mbox-cells Shall be 1 - the index of the channel needed. >> +- #mbox-cells Shall be 1 - the index of the channel needed when >> + compatible is "arm,mhu" >> + Shall be 2 - the index of the channel needed, and >> + the index of the doorbell bit with the channel >> when >> + compatible is "arm,mhu-doorbell" >> - interrupts: Contains the interrupt information corresponding >> to >> - each of the 3 links of MHU. >> + each of the 3 physical channels of MHU namely low >> + priority non-secure, high priority non-secure and >> + secure channels. >> >> Example: >> >> >> +1. Controller which doesn't support doorbells >> + >> mhu: mailbox@2b1f { >> #mbox-cells = <1>; >> compatible = "arm,mhu", "arm,primecell"; >> @@ -41,3 +62,22 @@ used by Linux running NS. >> reg = <0 0x2e00 0x4000>; >> mboxes = < 1>; /* HP-NonSecure */ >> }; >> + >> +2. Controller which supports doorbells >> + >> + mhu: mailbox@2b1f { >> + #mbox-cells = <2>; >> + compatible = "arm,mhu-doorbell", "arm,primecell"; >> + reg = <0 0x2b1f 0x1000>; >> + interrupts = <0 36 4>, /* LP-NonSecure */ >> +<0 35 4>, /* HP-NonSecure */ >> +<0 37 4>; /* Secure */ >> + clocks = < 0 2 1>; >> + clock-names = "apb_pclk"; >> + }; >> + >> + mhu_client: scb@2e00 { >> + compatible = "arm,scpi"; >> + reg = <0 0x2e00 0x200>; >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ >> + }; >> > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" > equally fine. So you are basically smuggling a s/w feature into DT. > I disagree, the spec clearly says each bit can be used for different event and hence we need a way to specify that in DT when used as doorbell. >>> >>> I think the point is that you should not continue to use both. The >>> >> FYKI my point (here and in other threads) is that current MHU >> driver/bindings can very well support Sudeep's usecase. >> >>> single cell usage should be deprecated. Maybe you'll have to encode the >>> 2nd cell when not used as 0 means bit 0? >>> >>> Arguably, you don't even need a
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 02/06/17 10:32, Sudeep Holla wrote: > > > On 02/06/17 06:45, Jassi Brar wrote: >> Hi Rob, >> >> On Wed, May 31, 2017 at 10:38 PM, Rob Herring wrote: >>> On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 >> -- >> 1 file changed, 43 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> index 4971f03f0b33..bd9a3a267caf 100644 >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having >> read the data. >> The last channel is specified to be a 'Secure' resource, hence can't be >> used by Linux running NS. >> >> +The MHU drives the interrupt signal using a 32-bit register, with all >> +32-bits logically ORed together. It provides a set of registers to >> +enable software to set, clear and check the status of each of the bits >> +of this register independently. The use of 32 bits per interrupt line >> +enables software to provide more information about the source of the >> +interrupt. For example, each bit of the register can be associated with >> +a type of event that can contribute to raising the interrupt. Each of >> +the 32-bits can be used as "doorbell" to alert the remote processor. >> + >> Mailbox Device Node: >> >> >> Required properties: >> >> -- compatible: Shall be "arm,mhu" & "arm,primecell" >> +- compatible: Shall be "arm,primecell" and one of the below: >> + "arm,mhu" - if the controller doesn't support >> + doorbell model >> + "arm,mhu-doorbell" - if the controller supports >> + doorbell model >> - reg: Contains the mailbox register address range (base >> address and length) >> -- #mbox-cells Shall be 1 - the index of the channel needed. >> +- #mbox-cells Shall be 1 - the index of the channel needed when >> + compatible is "arm,mhu" >> + Shall be 2 - the index of the channel needed, and >> + the index of the doorbell bit with the channel >> when >> + compatible is "arm,mhu-doorbell" >> - interrupts: Contains the interrupt information corresponding >> to >> - each of the 3 links of MHU. >> + each of the 3 physical channels of MHU namely low >> + priority non-secure, high priority non-secure and >> + secure channels. >> >> Example: >> >> >> +1. Controller which doesn't support doorbells >> + >> mhu: mailbox@2b1f { >> #mbox-cells = <1>; >> compatible = "arm,mhu", "arm,primecell"; >> @@ -41,3 +62,22 @@ used by Linux running NS. >> reg = <0 0x2e00 0x4000>; >> mboxes = < 1>; /* HP-NonSecure */ >> }; >> + >> +2. Controller which supports doorbells >> + >> + mhu: mailbox@2b1f { >> + #mbox-cells = <2>; >> + compatible = "arm,mhu-doorbell", "arm,primecell"; >> + reg = <0 0x2b1f 0x1000>; >> + interrupts = <0 36 4>, /* LP-NonSecure */ >> +<0 35 4>, /* HP-NonSecure */ >> +<0 37 4>; /* Secure */ >> + clocks = < 0 2 1>; >> + clock-names = "apb_pclk"; >> + }; >> + >> + mhu_client: scb@2e00 { >> + compatible = "arm,scpi"; >> + reg = <0 0x2e00 0x200>; >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ >> + }; >> > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" > equally fine. So you are basically smuggling a s/w feature into DT. > I disagree, the spec clearly says each bit can be used for different event and hence we need a way to specify that in DT when used as doorbell. >>> >>> I think the point is that you should not continue to use both. The >>> >> FYKI my point (here and in other threads) is that current MHU >> driver/bindings can very well support Sudeep's usecase. >> >>> single cell usage should be deprecated. Maybe you'll have to encode the >>> 2nd cell when not used as 0 means bit 0? >>> >>> Arguably, you don't even need a new compatible.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 02/06/17 06:45, Jassi Brar wrote: > Hi Rob, > > On Wed, May 31, 2017 at 10:38 PM, Rob Herringwrote: >> On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >>> > .../devicetree/bindings/mailbox/arm-mhu.txt| 46 > -- > 1 file changed, 43 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > index 4971f03f0b33..bd9a3a267caf 100644 > --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > @@ -10,21 +10,42 @@ STAT register and the remote clears it after having > read the data. > The last channel is specified to be a 'Secure' resource, hence can't be > used by Linux running NS. > > +The MHU drives the interrupt signal using a 32-bit register, with all > +32-bits logically ORed together. It provides a set of registers to > +enable software to set, clear and check the status of each of the bits > +of this register independently. The use of 32 bits per interrupt line > +enables software to provide more information about the source of the > +interrupt. For example, each bit of the register can be associated with > +a type of event that can contribute to raising the interrupt. Each of > +the 32-bits can be used as "doorbell" to alert the remote processor. > + > Mailbox Device Node: > > > Required properties: > > -- compatible: Shall be "arm,mhu" & "arm,primecell" > +- compatible: Shall be "arm,primecell" and one of the below: > + "arm,mhu" - if the controller doesn't support > + doorbell model > + "arm,mhu-doorbell" - if the controller supports > + doorbell model > - reg: Contains the mailbox register address range (base > address and length) > -- #mbox-cells Shall be 1 - the index of the channel needed. > +- #mbox-cells Shall be 1 - the index of the channel needed when > + compatible is "arm,mhu" > + Shall be 2 - the index of the channel needed, and > + the index of the doorbell bit with the channel > when > + compatible is "arm,mhu-doorbell" > - interrupts: Contains the interrupt information corresponding > to > - each of the 3 links of MHU. > + each of the 3 physical channels of MHU namely low > + priority non-secure, high priority non-secure and > + secure channels. > > Example: > > > +1. Controller which doesn't support doorbells > + > mhu: mailbox@2b1f { > #mbox-cells = <1>; > compatible = "arm,mhu", "arm,primecell"; > @@ -41,3 +62,22 @@ used by Linux running NS. > reg = <0 0x2e00 0x4000>; > mboxes = < 1>; /* HP-NonSecure */ > }; > + > +2. Controller which supports doorbells > + > + mhu: mailbox@2b1f { > + #mbox-cells = <2>; > + compatible = "arm,mhu-doorbell", "arm,primecell"; > + reg = <0 0x2b1f 0x1000>; > + interrupts = <0 36 4>, /* LP-NonSecure */ > +<0 35 4>, /* HP-NonSecure */ > +<0 37 4>; /* Secure */ > + clocks = < 0 2 1>; > + clock-names = "apb_pclk"; > + }; > + > + mhu_client: scb@2e00 { > + compatible = "arm,scpi"; > + reg = <0 0x2e00 0x200>; > + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ > + }; > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" equally fine. So you are basically smuggling a s/w feature into DT. >>> >>> I disagree, the spec clearly says each bit can be used for different >>> event and hence we need a way to specify that in DT when used as doorbell. >> >> I think the point is that you should not continue to use both. The >> > FYKI my point (here and in other threads) is that current MHU > driver/bindings can very well support Sudeep's usecase. > >> single cell usage should be deprecated. Maybe you'll have to encode the >> 2nd cell when not used as 0 means bit 0? >> >> Arguably, you don't even need a new compatible. #mbox-cells tells you >> whether to use the old or new binding. I'm fine either way though. >> > In mailbox terminology, there is a
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 02/06/17 06:45, Jassi Brar wrote: > Hi Rob, > > On Wed, May 31, 2017 at 10:38 PM, Rob Herring wrote: >> On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >>> > .../devicetree/bindings/mailbox/arm-mhu.txt| 46 > -- > 1 file changed, 43 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > index 4971f03f0b33..bd9a3a267caf 100644 > --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > @@ -10,21 +10,42 @@ STAT register and the remote clears it after having > read the data. > The last channel is specified to be a 'Secure' resource, hence can't be > used by Linux running NS. > > +The MHU drives the interrupt signal using a 32-bit register, with all > +32-bits logically ORed together. It provides a set of registers to > +enable software to set, clear and check the status of each of the bits > +of this register independently. The use of 32 bits per interrupt line > +enables software to provide more information about the source of the > +interrupt. For example, each bit of the register can be associated with > +a type of event that can contribute to raising the interrupt. Each of > +the 32-bits can be used as "doorbell" to alert the remote processor. > + > Mailbox Device Node: > > > Required properties: > > -- compatible: Shall be "arm,mhu" & "arm,primecell" > +- compatible: Shall be "arm,primecell" and one of the below: > + "arm,mhu" - if the controller doesn't support > + doorbell model > + "arm,mhu-doorbell" - if the controller supports > + doorbell model > - reg: Contains the mailbox register address range (base > address and length) > -- #mbox-cells Shall be 1 - the index of the channel needed. > +- #mbox-cells Shall be 1 - the index of the channel needed when > + compatible is "arm,mhu" > + Shall be 2 - the index of the channel needed, and > + the index of the doorbell bit with the channel > when > + compatible is "arm,mhu-doorbell" > - interrupts: Contains the interrupt information corresponding > to > - each of the 3 links of MHU. > + each of the 3 physical channels of MHU namely low > + priority non-secure, high priority non-secure and > + secure channels. > > Example: > > > +1. Controller which doesn't support doorbells > + > mhu: mailbox@2b1f { > #mbox-cells = <1>; > compatible = "arm,mhu", "arm,primecell"; > @@ -41,3 +62,22 @@ used by Linux running NS. > reg = <0 0x2e00 0x4000>; > mboxes = < 1>; /* HP-NonSecure */ > }; > + > +2. Controller which supports doorbells > + > + mhu: mailbox@2b1f { > + #mbox-cells = <2>; > + compatible = "arm,mhu-doorbell", "arm,primecell"; > + reg = <0 0x2b1f 0x1000>; > + interrupts = <0 36 4>, /* LP-NonSecure */ > +<0 35 4>, /* HP-NonSecure */ > +<0 37 4>; /* Secure */ > + clocks = < 0 2 1>; > + clock-names = "apb_pclk"; > + }; > + > + mhu_client: scb@2e00 { > + compatible = "arm,scpi"; > + reg = <0 0x2e00 0x200>; > + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ > + }; > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" equally fine. So you are basically smuggling a s/w feature into DT. >>> >>> I disagree, the spec clearly says each bit can be used for different >>> event and hence we need a way to specify that in DT when used as doorbell. >> >> I think the point is that you should not continue to use both. The >> > FYKI my point (here and in other threads) is that current MHU > driver/bindings can very well support Sudeep's usecase. > >> single cell usage should be deprecated. Maybe you'll have to encode the >> 2nd cell when not used as 0 means bit 0? >> >> Arguably, you don't even need a new compatible. #mbox-cells tells you >> whether to use the old or new binding. I'm fine either way though. >> > In mailbox terminology, there is a channel and
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
Hi Rob, On Wed, May 31, 2017 at 10:38 PM, Rob Herringwrote: > On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >> >> >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 >> >> -- >> >> 1 file changed, 43 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> >> index 4971f03f0b33..bd9a3a267caf 100644 >> >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having >> >> read the data. >> >> The last channel is specified to be a 'Secure' resource, hence can't be >> >> used by Linux running NS. >> >> >> >> +The MHU drives the interrupt signal using a 32-bit register, with all >> >> +32-bits logically ORed together. It provides a set of registers to >> >> +enable software to set, clear and check the status of each of the bits >> >> +of this register independently. The use of 32 bits per interrupt line >> >> +enables software to provide more information about the source of the >> >> +interrupt. For example, each bit of the register can be associated with >> >> +a type of event that can contribute to raising the interrupt. Each of >> >> +the 32-bits can be used as "doorbell" to alert the remote processor. >> >> + >> >> Mailbox Device Node: >> >> >> >> >> >> Required properties: >> >> >> >> -- compatible: Shall be "arm,mhu" & "arm,primecell" >> >> +- compatible: Shall be "arm,primecell" and one of the below: >> >> + "arm,mhu" - if the controller doesn't support >> >> + doorbell model >> >> + "arm,mhu-doorbell" - if the controller supports >> >> + doorbell model >> >> - reg: Contains the mailbox register address range (base >> >> address and length) >> >> -- #mbox-cells Shall be 1 - the index of the channel needed. >> >> +- #mbox-cells Shall be 1 - the index of the channel needed when >> >> + compatible is "arm,mhu" >> >> + Shall be 2 - the index of the channel needed, and >> >> + the index of the doorbell bit with the channel >> >> when >> >> + compatible is "arm,mhu-doorbell" >> >> - interrupts: Contains the interrupt information corresponding >> >> to >> >> - each of the 3 links of MHU. >> >> + each of the 3 physical channels of MHU namely low >> >> + priority non-secure, high priority non-secure and >> >> + secure channels. >> >> >> >> Example: >> >> >> >> >> >> +1. Controller which doesn't support doorbells >> >> + >> >> mhu: mailbox@2b1f { >> >> #mbox-cells = <1>; >> >> compatible = "arm,mhu", "arm,primecell"; >> >> @@ -41,3 +62,22 @@ used by Linux running NS. >> >> reg = <0 0x2e00 0x4000>; >> >> mboxes = < 1>; /* HP-NonSecure */ >> >> }; >> >> + >> >> +2. Controller which supports doorbells >> >> + >> >> + mhu: mailbox@2b1f { >> >> + #mbox-cells = <2>; >> >> + compatible = "arm,mhu-doorbell", "arm,primecell"; >> >> + reg = <0 0x2b1f 0x1000>; >> >> + interrupts = <0 36 4>, /* LP-NonSecure */ >> >> +<0 35 4>, /* HP-NonSecure */ >> >> +<0 37 4>; /* Secure */ >> >> + clocks = < 0 2 1>; >> >> + clock-names = "apb_pclk"; >> >> + }; >> >> + >> >> + mhu_client: scb@2e00 { >> >> + compatible = "arm,scpi"; >> >> + reg = <0 0x2e00 0x200>; >> >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ >> >> + }; >> >> >> > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" >> > equally fine. So you are basically smuggling a s/w feature into DT. >> > >> >> I disagree, the spec clearly says each bit can be used for different >> event and hence we need a way to specify that in DT when used as doorbell. > > I think the point is that you should not continue to use both. The > FYKI my point (here and in other threads) is that current MHU driver/bindings can very well support Sudeep's usecase. > single cell usage should be deprecated. Maybe you'll have to encode the > 2nd cell when not used as 0 means bit 0? > > Arguably, you don't even need a new compatible. #mbox-cells tells you > whether to use the old or new binding. I'm fine either way though. > In mailbox terminology, there is a channel and command(s) that we send/recv over it. OOB data passing is
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
Hi Rob, On Wed, May 31, 2017 at 10:38 PM, Rob Herring wrote: > On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >> >> >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 >> >> -- >> >> 1 file changed, 43 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> >> index 4971f03f0b33..bd9a3a267caf 100644 >> >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having >> >> read the data. >> >> The last channel is specified to be a 'Secure' resource, hence can't be >> >> used by Linux running NS. >> >> >> >> +The MHU drives the interrupt signal using a 32-bit register, with all >> >> +32-bits logically ORed together. It provides a set of registers to >> >> +enable software to set, clear and check the status of each of the bits >> >> +of this register independently. The use of 32 bits per interrupt line >> >> +enables software to provide more information about the source of the >> >> +interrupt. For example, each bit of the register can be associated with >> >> +a type of event that can contribute to raising the interrupt. Each of >> >> +the 32-bits can be used as "doorbell" to alert the remote processor. >> >> + >> >> Mailbox Device Node: >> >> >> >> >> >> Required properties: >> >> >> >> -- compatible: Shall be "arm,mhu" & "arm,primecell" >> >> +- compatible: Shall be "arm,primecell" and one of the below: >> >> + "arm,mhu" - if the controller doesn't support >> >> + doorbell model >> >> + "arm,mhu-doorbell" - if the controller supports >> >> + doorbell model >> >> - reg: Contains the mailbox register address range (base >> >> address and length) >> >> -- #mbox-cells Shall be 1 - the index of the channel needed. >> >> +- #mbox-cells Shall be 1 - the index of the channel needed when >> >> + compatible is "arm,mhu" >> >> + Shall be 2 - the index of the channel needed, and >> >> + the index of the doorbell bit with the channel >> >> when >> >> + compatible is "arm,mhu-doorbell" >> >> - interrupts: Contains the interrupt information corresponding >> >> to >> >> - each of the 3 links of MHU. >> >> + each of the 3 physical channels of MHU namely low >> >> + priority non-secure, high priority non-secure and >> >> + secure channels. >> >> >> >> Example: >> >> >> >> >> >> +1. Controller which doesn't support doorbells >> >> + >> >> mhu: mailbox@2b1f { >> >> #mbox-cells = <1>; >> >> compatible = "arm,mhu", "arm,primecell"; >> >> @@ -41,3 +62,22 @@ used by Linux running NS. >> >> reg = <0 0x2e00 0x4000>; >> >> mboxes = < 1>; /* HP-NonSecure */ >> >> }; >> >> + >> >> +2. Controller which supports doorbells >> >> + >> >> + mhu: mailbox@2b1f { >> >> + #mbox-cells = <2>; >> >> + compatible = "arm,mhu-doorbell", "arm,primecell"; >> >> + reg = <0 0x2b1f 0x1000>; >> >> + interrupts = <0 36 4>, /* LP-NonSecure */ >> >> +<0 35 4>, /* HP-NonSecure */ >> >> +<0 37 4>; /* Secure */ >> >> + clocks = < 0 2 1>; >> >> + clock-names = "apb_pclk"; >> >> + }; >> >> + >> >> + mhu_client: scb@2e00 { >> >> + compatible = "arm,scpi"; >> >> + reg = <0 0x2e00 0x200>; >> >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ >> >> + }; >> >> >> > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" >> > equally fine. So you are basically smuggling a s/w feature into DT. >> > >> >> I disagree, the spec clearly says each bit can be used for different >> event and hence we need a way to specify that in DT when used as doorbell. > > I think the point is that you should not continue to use both. The > FYKI my point (here and in other threads) is that current MHU driver/bindings can very well support Sudeep's usecase. > single cell usage should be deprecated. Maybe you'll have to encode the > 2nd cell when not used as 0 means bit 0? > > Arguably, you don't even need a new compatible. #mbox-cells tells you > whether to use the old or new binding. I'm fine either way though. > In mailbox terminology, there is a channel and command(s) that we send/recv over it. OOB data passing is optional. MHU
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 31/05/17 18:08, Rob Herring wrote: > On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >> >> >> On 25/05/17 14:22, Jassi Brar wrote: [...] >>> Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" >>> equally fine. So you are basically smuggling a s/w feature into DT. >>> >> >> I disagree, the spec clearly says each bit can be used for different >> event and hence we need a way to specify that in DT when used as doorbell. > > I think the point is that you should not continue to use both. The > single cell usage should be deprecated. Maybe you'll have to encode the > 2nd cell when not used as 0 means bit 0? > Instead of having special encoding, I like your below suggestion on using #mbox-cells to distinguish the usage modes. > Arguably, you don't even need a new compatible. #mbox-cells tells you > whether to use the old or new binding. I'm fine either way though. > Ah good point, yes we can distinguish with #mbox-cells. I will drop the new compatible. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 31/05/17 18:08, Rob Herring wrote: > On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: >> >> >> On 25/05/17 14:22, Jassi Brar wrote: [...] >>> Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" >>> equally fine. So you are basically smuggling a s/w feature into DT. >>> >> >> I disagree, the spec clearly says each bit can be used for different >> event and hence we need a way to specify that in DT when used as doorbell. > > I think the point is that you should not continue to use both. The > single cell usage should be deprecated. Maybe you'll have to encode the > 2nd cell when not used as 0 means bit 0? > Instead of having special encoding, I like your below suggestion on using #mbox-cells to distinguish the usage modes. > Arguably, you don't even need a new compatible. #mbox-cells tells you > whether to use the old or new binding. I'm fine either way though. > Ah good point, yes we can distinguish with #mbox-cells. I will drop the new compatible. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: > > > On 25/05/17 14:22, Jassi Brar wrote: > > On Wed, May 24, 2017 at 3:46 PM, Sudeep Hollawrote: > >> The ARM MHU has mechanism to assert interrupt signals to facilitate > >> inter-processor message based communication. It drives the signal using > >> a 32-bit register, with all 32-bits logically ORed together. It also > >> enables software to set, clear and check the status of each of the bits > >> of this register independently. Each bit of the register can be > >> associated with a type of event that can contribute to raising the > >> interrupt thereby allowing it to be used as independent doorbells. > >> > >> Since the first version of this binding can't support doorbells, > >> this patch extends the existing binding to support them. > >> > >> Cc: Alexey Klimov > >> Cc: Jassi Brar > >> Cc: Rob Herring > >> Cc: devicet...@vger.kernel.org > >> Signed-off-by: Sudeep Holla > >> --- > >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 > >> -- > >> 1 file changed, 43 insertions(+), 3 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > >> index 4971f03f0b33..bd9a3a267caf 100644 > >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having > >> read the data. > >> The last channel is specified to be a 'Secure' resource, hence can't be > >> used by Linux running NS. > >> > >> +The MHU drives the interrupt signal using a 32-bit register, with all > >> +32-bits logically ORed together. It provides a set of registers to > >> +enable software to set, clear and check the status of each of the bits > >> +of this register independently. The use of 32 bits per interrupt line > >> +enables software to provide more information about the source of the > >> +interrupt. For example, each bit of the register can be associated with > >> +a type of event that can contribute to raising the interrupt. Each of > >> +the 32-bits can be used as "doorbell" to alert the remote processor. > >> + > >> Mailbox Device Node: > >> > >> > >> Required properties: > >> > >> -- compatible: Shall be "arm,mhu" & "arm,primecell" > >> +- compatible: Shall be "arm,primecell" and one of the below: > >> + "arm,mhu" - if the controller doesn't support > >> + doorbell model > >> + "arm,mhu-doorbell" - if the controller supports > >> + doorbell model > >> - reg: Contains the mailbox register address range (base > >> address and length) > >> -- #mbox-cells Shall be 1 - the index of the channel needed. > >> +- #mbox-cells Shall be 1 - the index of the channel needed when > >> + compatible is "arm,mhu" > >> + Shall be 2 - the index of the channel needed, and > >> + the index of the doorbell bit with the channel when > >> + compatible is "arm,mhu-doorbell" > >> - interrupts: Contains the interrupt information corresponding to > >> - each of the 3 links of MHU. > >> + each of the 3 physical channels of MHU namely low > >> + priority non-secure, high priority non-secure and > >> + secure channels. > >> > >> Example: > >> > >> > >> +1. Controller which doesn't support doorbells > >> + > >> mhu: mailbox@2b1f { > >> #mbox-cells = <1>; > >> compatible = "arm,mhu", "arm,primecell"; > >> @@ -41,3 +62,22 @@ used by Linux running NS. > >> reg = <0 0x2e00 0x4000>; > >> mboxes = < 1>; /* HP-NonSecure */ > >> }; > >> + > >> +2. Controller which supports doorbells > >> + > >> + mhu: mailbox@2b1f { > >> + #mbox-cells = <2>; > >> + compatible = "arm,mhu-doorbell", "arm,primecell"; > >> + reg = <0 0x2b1f 0x1000>; > >> + interrupts = <0 36 4>, /* LP-NonSecure */ > >> +<0 35 4>, /* HP-NonSecure */ > >> +<0 37 4>; /* Secure */ > >> + clocks = < 0 2 1>; > >> + clock-names = "apb_pclk"; > >> + }; > >> + > >> + mhu_client: scb@2e00 { > >> + compatible = "arm,scpi"; > >> + reg = <0 0x2e00 0x200>; > >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ > >> + }; > >> > > Every MHU
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Thu, May 25, 2017 at 02:23:44PM +0100, Sudeep Holla wrote: > > > On 25/05/17 14:22, Jassi Brar wrote: > > On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla wrote: > >> The ARM MHU has mechanism to assert interrupt signals to facilitate > >> inter-processor message based communication. It drives the signal using > >> a 32-bit register, with all 32-bits logically ORed together. It also > >> enables software to set, clear and check the status of each of the bits > >> of this register independently. Each bit of the register can be > >> associated with a type of event that can contribute to raising the > >> interrupt thereby allowing it to be used as independent doorbells. > >> > >> Since the first version of this binding can't support doorbells, > >> this patch extends the existing binding to support them. > >> > >> Cc: Alexey Klimov > >> Cc: Jassi Brar > >> Cc: Rob Herring > >> Cc: devicet...@vger.kernel.org > >> Signed-off-by: Sudeep Holla > >> --- > >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 > >> -- > >> 1 file changed, 43 insertions(+), 3 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > >> index 4971f03f0b33..bd9a3a267caf 100644 > >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having > >> read the data. > >> The last channel is specified to be a 'Secure' resource, hence can't be > >> used by Linux running NS. > >> > >> +The MHU drives the interrupt signal using a 32-bit register, with all > >> +32-bits logically ORed together. It provides a set of registers to > >> +enable software to set, clear and check the status of each of the bits > >> +of this register independently. The use of 32 bits per interrupt line > >> +enables software to provide more information about the source of the > >> +interrupt. For example, each bit of the register can be associated with > >> +a type of event that can contribute to raising the interrupt. Each of > >> +the 32-bits can be used as "doorbell" to alert the remote processor. > >> + > >> Mailbox Device Node: > >> > >> > >> Required properties: > >> > >> -- compatible: Shall be "arm,mhu" & "arm,primecell" > >> +- compatible: Shall be "arm,primecell" and one of the below: > >> + "arm,mhu" - if the controller doesn't support > >> + doorbell model > >> + "arm,mhu-doorbell" - if the controller supports > >> + doorbell model > >> - reg: Contains the mailbox register address range (base > >> address and length) > >> -- #mbox-cells Shall be 1 - the index of the channel needed. > >> +- #mbox-cells Shall be 1 - the index of the channel needed when > >> + compatible is "arm,mhu" > >> + Shall be 2 - the index of the channel needed, and > >> + the index of the doorbell bit with the channel when > >> + compatible is "arm,mhu-doorbell" > >> - interrupts: Contains the interrupt information corresponding to > >> - each of the 3 links of MHU. > >> + each of the 3 physical channels of MHU namely low > >> + priority non-secure, high priority non-secure and > >> + secure channels. > >> > >> Example: > >> > >> > >> +1. Controller which doesn't support doorbells > >> + > >> mhu: mailbox@2b1f { > >> #mbox-cells = <1>; > >> compatible = "arm,mhu", "arm,primecell"; > >> @@ -41,3 +62,22 @@ used by Linux running NS. > >> reg = <0 0x2e00 0x4000>; > >> mboxes = < 1>; /* HP-NonSecure */ > >> }; > >> + > >> +2. Controller which supports doorbells > >> + > >> + mhu: mailbox@2b1f { > >> + #mbox-cells = <2>; > >> + compatible = "arm,mhu-doorbell", "arm,primecell"; > >> + reg = <0 0x2b1f 0x1000>; > >> + interrupts = <0 36 4>, /* LP-NonSecure */ > >> +<0 35 4>, /* HP-NonSecure */ > >> +<0 37 4>; /* Secure */ > >> + clocks = < 0 2 1>; > >> + clock-names = "apb_pclk"; > >> + }; > >> + > >> + mhu_client: scb@2e00 { > >> + compatible = "arm,scpi"; > >> + reg = <0 0x2e00 0x200>; > >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ > >> + }; > >> > > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" > > equally fine. So you are basically smuggling a s/w feature
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 25/05/17 14:22, Jassi Brar wrote: > On Wed, May 24, 2017 at 3:46 PM, Sudeep Hollawrote: >> The ARM MHU has mechanism to assert interrupt signals to facilitate >> inter-processor message based communication. It drives the signal using >> a 32-bit register, with all 32-bits logically ORed together. It also >> enables software to set, clear and check the status of each of the bits >> of this register independently. Each bit of the register can be >> associated with a type of event that can contribute to raising the >> interrupt thereby allowing it to be used as independent doorbells. >> >> Since the first version of this binding can't support doorbells, >> this patch extends the existing binding to support them. >> >> Cc: Alexey Klimov >> Cc: Jassi Brar >> Cc: Rob Herring >> Cc: devicet...@vger.kernel.org >> Signed-off-by: Sudeep Holla >> --- >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 >> -- >> 1 file changed, 43 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> index 4971f03f0b33..bd9a3a267caf 100644 >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read >> the data. >> The last channel is specified to be a 'Secure' resource, hence can't be >> used by Linux running NS. >> >> +The MHU drives the interrupt signal using a 32-bit register, with all >> +32-bits logically ORed together. It provides a set of registers to >> +enable software to set, clear and check the status of each of the bits >> +of this register independently. The use of 32 bits per interrupt line >> +enables software to provide more information about the source of the >> +interrupt. For example, each bit of the register can be associated with >> +a type of event that can contribute to raising the interrupt. Each of >> +the 32-bits can be used as "doorbell" to alert the remote processor. >> + >> Mailbox Device Node: >> >> >> Required properties: >> >> -- compatible: Shall be "arm,mhu" & "arm,primecell" >> +- compatible: Shall be "arm,primecell" and one of the below: >> + "arm,mhu" - if the controller doesn't support >> + doorbell model >> + "arm,mhu-doorbell" - if the controller supports >> + doorbell model >> - reg: Contains the mailbox register address range (base >> address and length) >> -- #mbox-cells Shall be 1 - the index of the channel needed. >> +- #mbox-cells Shall be 1 - the index of the channel needed when >> + compatible is "arm,mhu" >> + Shall be 2 - the index of the channel needed, and >> + the index of the doorbell bit with the channel when >> + compatible is "arm,mhu-doorbell" >> - interrupts: Contains the interrupt information corresponding to >> - each of the 3 links of MHU. >> + each of the 3 physical channels of MHU namely low >> + priority non-secure, high priority non-secure and >> + secure channels. >> >> Example: >> >> >> +1. Controller which doesn't support doorbells >> + >> mhu: mailbox@2b1f { >> #mbox-cells = <1>; >> compatible = "arm,mhu", "arm,primecell"; >> @@ -41,3 +62,22 @@ used by Linux running NS. >> reg = <0 0x2e00 0x4000>; >> mboxes = < 1>; /* HP-NonSecure */ >> }; >> + >> +2. Controller which supports doorbells >> + >> + mhu: mailbox@2b1f { >> + #mbox-cells = <2>; >> + compatible = "arm,mhu-doorbell", "arm,primecell"; >> + reg = <0 0x2b1f 0x1000>; >> + interrupts = <0 36 4>, /* LP-NonSecure */ >> +<0 35 4>, /* HP-NonSecure */ >> +<0 37 4>; /* Secure */ >> + clocks = < 0 2 1>; >> + clock-names = "apb_pclk"; >> + }; >> + >> + mhu_client: scb@2e00 { >> + compatible = "arm,scpi"; >> + reg = <0 0x2e00 0x200>; >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ >> + }; >> > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" > equally fine. So you are basically smuggling a s/w feature into DT. > I have asked many times now, please explain how would you handle multiple protocol on a single set of MHU registers. BIT(0) - SCPI protocol
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 25/05/17 14:22, Jassi Brar wrote: > On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla wrote: >> The ARM MHU has mechanism to assert interrupt signals to facilitate >> inter-processor message based communication. It drives the signal using >> a 32-bit register, with all 32-bits logically ORed together. It also >> enables software to set, clear and check the status of each of the bits >> of this register independently. Each bit of the register can be >> associated with a type of event that can contribute to raising the >> interrupt thereby allowing it to be used as independent doorbells. >> >> Since the first version of this binding can't support doorbells, >> this patch extends the existing binding to support them. >> >> Cc: Alexey Klimov >> Cc: Jassi Brar >> Cc: Rob Herring >> Cc: devicet...@vger.kernel.org >> Signed-off-by: Sudeep Holla >> --- >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 >> -- >> 1 file changed, 43 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> index 4971f03f0b33..bd9a3a267caf 100644 >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read >> the data. >> The last channel is specified to be a 'Secure' resource, hence can't be >> used by Linux running NS. >> >> +The MHU drives the interrupt signal using a 32-bit register, with all >> +32-bits logically ORed together. It provides a set of registers to >> +enable software to set, clear and check the status of each of the bits >> +of this register independently. The use of 32 bits per interrupt line >> +enables software to provide more information about the source of the >> +interrupt. For example, each bit of the register can be associated with >> +a type of event that can contribute to raising the interrupt. Each of >> +the 32-bits can be used as "doorbell" to alert the remote processor. >> + >> Mailbox Device Node: >> >> >> Required properties: >> >> -- compatible: Shall be "arm,mhu" & "arm,primecell" >> +- compatible: Shall be "arm,primecell" and one of the below: >> + "arm,mhu" - if the controller doesn't support >> + doorbell model >> + "arm,mhu-doorbell" - if the controller supports >> + doorbell model >> - reg: Contains the mailbox register address range (base >> address and length) >> -- #mbox-cells Shall be 1 - the index of the channel needed. >> +- #mbox-cells Shall be 1 - the index of the channel needed when >> + compatible is "arm,mhu" >> + Shall be 2 - the index of the channel needed, and >> + the index of the doorbell bit with the channel when >> + compatible is "arm,mhu-doorbell" >> - interrupts: Contains the interrupt information corresponding to >> - each of the 3 links of MHU. >> + each of the 3 physical channels of MHU namely low >> + priority non-secure, high priority non-secure and >> + secure channels. >> >> Example: >> >> >> +1. Controller which doesn't support doorbells >> + >> mhu: mailbox@2b1f { >> #mbox-cells = <1>; >> compatible = "arm,mhu", "arm,primecell"; >> @@ -41,3 +62,22 @@ used by Linux running NS. >> reg = <0 0x2e00 0x4000>; >> mboxes = < 1>; /* HP-NonSecure */ >> }; >> + >> +2. Controller which supports doorbells >> + >> + mhu: mailbox@2b1f { >> + #mbox-cells = <2>; >> + compatible = "arm,mhu-doorbell", "arm,primecell"; >> + reg = <0 0x2b1f 0x1000>; >> + interrupts = <0 36 4>, /* LP-NonSecure */ >> +<0 35 4>, /* HP-NonSecure */ >> +<0 37 4>; /* Secure */ >> + clocks = < 0 2 1>; >> + clock-names = "apb_pclk"; >> + }; >> + >> + mhu_client: scb@2e00 { >> + compatible = "arm,scpi"; >> + reg = <0 0x2e00 0x200>; >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ >> + }; >> > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" > equally fine. So you are basically smuggling a s/w feature into DT. > I have asked many times now, please explain how would you handle multiple protocol on a single set of MHU registers. BIT(0) - SCPI protocol BIT(1) - SCMI (general) BIT(2) - SCMI (notification) BIT(3) - A totally new protocol : : with each of the above
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 25/05/17 14:22, Jassi Brar wrote: > On Wed, May 24, 2017 at 3:46 PM, Sudeep Hollawrote: >> The ARM MHU has mechanism to assert interrupt signals to facilitate >> inter-processor message based communication. It drives the signal using >> a 32-bit register, with all 32-bits logically ORed together. It also >> enables software to set, clear and check the status of each of the bits >> of this register independently. Each bit of the register can be >> associated with a type of event that can contribute to raising the >> interrupt thereby allowing it to be used as independent doorbells. >> >> Since the first version of this binding can't support doorbells, >> this patch extends the existing binding to support them. >> >> Cc: Alexey Klimov >> Cc: Jassi Brar >> Cc: Rob Herring >> Cc: devicet...@vger.kernel.org >> Signed-off-by: Sudeep Holla >> --- >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 >> -- >> 1 file changed, 43 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> index 4971f03f0b33..bd9a3a267caf 100644 >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read >> the data. >> The last channel is specified to be a 'Secure' resource, hence can't be >> used by Linux running NS. >> >> +The MHU drives the interrupt signal using a 32-bit register, with all >> +32-bits logically ORed together. It provides a set of registers to >> +enable software to set, clear and check the status of each of the bits >> +of this register independently. The use of 32 bits per interrupt line >> +enables software to provide more information about the source of the >> +interrupt. For example, each bit of the register can be associated with >> +a type of event that can contribute to raising the interrupt. Each of >> +the 32-bits can be used as "doorbell" to alert the remote processor. >> + >> Mailbox Device Node: >> >> >> Required properties: >> >> -- compatible: Shall be "arm,mhu" & "arm,primecell" >> +- compatible: Shall be "arm,primecell" and one of the below: >> + "arm,mhu" - if the controller doesn't support >> + doorbell model >> + "arm,mhu-doorbell" - if the controller supports >> + doorbell model >> - reg: Contains the mailbox register address range (base >> address and length) >> -- #mbox-cells Shall be 1 - the index of the channel needed. >> +- #mbox-cells Shall be 1 - the index of the channel needed when >> + compatible is "arm,mhu" >> + Shall be 2 - the index of the channel needed, and >> + the index of the doorbell bit with the channel when >> + compatible is "arm,mhu-doorbell" >> - interrupts: Contains the interrupt information corresponding to >> - each of the 3 links of MHU. >> + each of the 3 physical channels of MHU namely low >> + priority non-secure, high priority non-secure and >> + secure channels. >> >> Example: >> >> >> +1. Controller which doesn't support doorbells >> + >> mhu: mailbox@2b1f { >> #mbox-cells = <1>; >> compatible = "arm,mhu", "arm,primecell"; >> @@ -41,3 +62,22 @@ used by Linux running NS. >> reg = <0 0x2e00 0x4000>; >> mboxes = < 1>; /* HP-NonSecure */ >> }; >> + >> +2. Controller which supports doorbells >> + >> + mhu: mailbox@2b1f { >> + #mbox-cells = <2>; >> + compatible = "arm,mhu-doorbell", "arm,primecell"; >> + reg = <0 0x2b1f 0x1000>; >> + interrupts = <0 36 4>, /* LP-NonSecure */ >> +<0 35 4>, /* HP-NonSecure */ >> +<0 37 4>; /* Secure */ >> + clocks = < 0 2 1>; >> + clock-names = "apb_pclk"; >> + }; >> + >> + mhu_client: scb@2e00 { >> + compatible = "arm,scpi"; >> + reg = <0 0x2e00 0x200>; >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ >> + }; >> > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" > equally fine. So you are basically smuggling a s/w feature into DT. > I disagree, the spec clearly says each bit can be used for different event and hence we need a way to specify that in DT when used as
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On 25/05/17 14:22, Jassi Brar wrote: > On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla wrote: >> The ARM MHU has mechanism to assert interrupt signals to facilitate >> inter-processor message based communication. It drives the signal using >> a 32-bit register, with all 32-bits logically ORed together. It also >> enables software to set, clear and check the status of each of the bits >> of this register independently. Each bit of the register can be >> associated with a type of event that can contribute to raising the >> interrupt thereby allowing it to be used as independent doorbells. >> >> Since the first version of this binding can't support doorbells, >> this patch extends the existing binding to support them. >> >> Cc: Alexey Klimov >> Cc: Jassi Brar >> Cc: Rob Herring >> Cc: devicet...@vger.kernel.org >> Signed-off-by: Sudeep Holla >> --- >> .../devicetree/bindings/mailbox/arm-mhu.txt| 46 >> -- >> 1 file changed, 43 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> index 4971f03f0b33..bd9a3a267caf 100644 >> --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt >> @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read >> the data. >> The last channel is specified to be a 'Secure' resource, hence can't be >> used by Linux running NS. >> >> +The MHU drives the interrupt signal using a 32-bit register, with all >> +32-bits logically ORed together. It provides a set of registers to >> +enable software to set, clear and check the status of each of the bits >> +of this register independently. The use of 32 bits per interrupt line >> +enables software to provide more information about the source of the >> +interrupt. For example, each bit of the register can be associated with >> +a type of event that can contribute to raising the interrupt. Each of >> +the 32-bits can be used as "doorbell" to alert the remote processor. >> + >> Mailbox Device Node: >> >> >> Required properties: >> >> -- compatible: Shall be "arm,mhu" & "arm,primecell" >> +- compatible: Shall be "arm,primecell" and one of the below: >> + "arm,mhu" - if the controller doesn't support >> + doorbell model >> + "arm,mhu-doorbell" - if the controller supports >> + doorbell model >> - reg: Contains the mailbox register address range (base >> address and length) >> -- #mbox-cells Shall be 1 - the index of the channel needed. >> +- #mbox-cells Shall be 1 - the index of the channel needed when >> + compatible is "arm,mhu" >> + Shall be 2 - the index of the channel needed, and >> + the index of the doorbell bit with the channel when >> + compatible is "arm,mhu-doorbell" >> - interrupts: Contains the interrupt information corresponding to >> - each of the 3 links of MHU. >> + each of the 3 physical channels of MHU namely low >> + priority non-secure, high priority non-secure and >> + secure channels. >> >> Example: >> >> >> +1. Controller which doesn't support doorbells >> + >> mhu: mailbox@2b1f { >> #mbox-cells = <1>; >> compatible = "arm,mhu", "arm,primecell"; >> @@ -41,3 +62,22 @@ used by Linux running NS. >> reg = <0 0x2e00 0x4000>; >> mboxes = < 1>; /* HP-NonSecure */ >> }; >> + >> +2. Controller which supports doorbells >> + >> + mhu: mailbox@2b1f { >> + #mbox-cells = <2>; >> + compatible = "arm,mhu-doorbell", "arm,primecell"; >> + reg = <0 0x2b1f 0x1000>; >> + interrupts = <0 36 4>, /* LP-NonSecure */ >> +<0 35 4>, /* HP-NonSecure */ >> +<0 37 4>; /* Secure */ >> + clocks = < 0 2 1>; >> + clock-names = "apb_pclk"; >> + }; >> + >> + mhu_client: scb@2e00 { >> + compatible = "arm,scpi"; >> + reg = <0 0x2e00 0x200>; >> + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ >> + }; >> > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" > equally fine. So you are basically smuggling a s/w feature into DT. > I disagree, the spec clearly says each bit can be used for different event and hence we need a way to specify that in DT when used as doorbell. -- Regards, Sudeep
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Wed, May 24, 2017 at 3:46 PM, Sudeep Hollawrote: > The ARM MHU has mechanism to assert interrupt signals to facilitate > inter-processor message based communication. It drives the signal using > a 32-bit register, with all 32-bits logically ORed together. It also > enables software to set, clear and check the status of each of the bits > of this register independently. Each bit of the register can be > associated with a type of event that can contribute to raising the > interrupt thereby allowing it to be used as independent doorbells. > > Since the first version of this binding can't support doorbells, > this patch extends the existing binding to support them. > > Cc: Alexey Klimov > Cc: Jassi Brar > Cc: Rob Herring > Cc: devicet...@vger.kernel.org > Signed-off-by: Sudeep Holla > --- > .../devicetree/bindings/mailbox/arm-mhu.txt| 46 > -- > 1 file changed, 43 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > index 4971f03f0b33..bd9a3a267caf 100644 > --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read > the data. > The last channel is specified to be a 'Secure' resource, hence can't be > used by Linux running NS. > > +The MHU drives the interrupt signal using a 32-bit register, with all > +32-bits logically ORed together. It provides a set of registers to > +enable software to set, clear and check the status of each of the bits > +of this register independently. The use of 32 bits per interrupt line > +enables software to provide more information about the source of the > +interrupt. For example, each bit of the register can be associated with > +a type of event that can contribute to raising the interrupt. Each of > +the 32-bits can be used as "doorbell" to alert the remote processor. > + > Mailbox Device Node: > > > Required properties: > > -- compatible: Shall be "arm,mhu" & "arm,primecell" > +- compatible: Shall be "arm,primecell" and one of the below: > + "arm,mhu" - if the controller doesn't support > + doorbell model > + "arm,mhu-doorbell" - if the controller supports > + doorbell model > - reg: Contains the mailbox register address range (base > address and length) > -- #mbox-cells Shall be 1 - the index of the channel needed. > +- #mbox-cells Shall be 1 - the index of the channel needed when > + compatible is "arm,mhu" > + Shall be 2 - the index of the channel needed, and > + the index of the doorbell bit with the channel when > + compatible is "arm,mhu-doorbell" > - interrupts: Contains the interrupt information corresponding to > - each of the 3 links of MHU. > + each of the 3 physical channels of MHU namely low > + priority non-secure, high priority non-secure and > + secure channels. > > Example: > > > +1. Controller which doesn't support doorbells > + > mhu: mailbox@2b1f { > #mbox-cells = <1>; > compatible = "arm,mhu", "arm,primecell"; > @@ -41,3 +62,22 @@ used by Linux running NS. > reg = <0 0x2e00 0x4000>; > mboxes = < 1>; /* HP-NonSecure */ > }; > + > +2. Controller which supports doorbells > + > + mhu: mailbox@2b1f { > + #mbox-cells = <2>; > + compatible = "arm,mhu-doorbell", "arm,primecell"; > + reg = <0 0x2b1f 0x1000>; > + interrupts = <0 36 4>, /* LP-NonSecure */ > +<0 35 4>, /* HP-NonSecure */ > +<0 37 4>; /* Secure */ > + clocks = < 0 2 1>; > + clock-names = "apb_pclk"; > + }; > + > + mhu_client: scb@2e00 { > + compatible = "arm,scpi"; > + reg = <0 0x2e00 0x200>; > + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ > + }; > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" equally fine. So you are basically smuggling a s/w feature into DT.
Re: [PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
On Wed, May 24, 2017 at 3:46 PM, Sudeep Holla wrote: > The ARM MHU has mechanism to assert interrupt signals to facilitate > inter-processor message based communication. It drives the signal using > a 32-bit register, with all 32-bits logically ORed together. It also > enables software to set, clear and check the status of each of the bits > of this register independently. Each bit of the register can be > associated with a type of event that can contribute to raising the > interrupt thereby allowing it to be used as independent doorbells. > > Since the first version of this binding can't support doorbells, > this patch extends the existing binding to support them. > > Cc: Alexey Klimov > Cc: Jassi Brar > Cc: Rob Herring > Cc: devicet...@vger.kernel.org > Signed-off-by: Sudeep Holla > --- > .../devicetree/bindings/mailbox/arm-mhu.txt| 46 > -- > 1 file changed, 43 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > index 4971f03f0b33..bd9a3a267caf 100644 > --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt > @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read > the data. > The last channel is specified to be a 'Secure' resource, hence can't be > used by Linux running NS. > > +The MHU drives the interrupt signal using a 32-bit register, with all > +32-bits logically ORed together. It provides a set of registers to > +enable software to set, clear and check the status of each of the bits > +of this register independently. The use of 32 bits per interrupt line > +enables software to provide more information about the source of the > +interrupt. For example, each bit of the register can be associated with > +a type of event that can contribute to raising the interrupt. Each of > +the 32-bits can be used as "doorbell" to alert the remote processor. > + > Mailbox Device Node: > > > Required properties: > > -- compatible: Shall be "arm,mhu" & "arm,primecell" > +- compatible: Shall be "arm,primecell" and one of the below: > + "arm,mhu" - if the controller doesn't support > + doorbell model > + "arm,mhu-doorbell" - if the controller supports > + doorbell model > - reg: Contains the mailbox register address range (base > address and length) > -- #mbox-cells Shall be 1 - the index of the channel needed. > +- #mbox-cells Shall be 1 - the index of the channel needed when > + compatible is "arm,mhu" > + Shall be 2 - the index of the channel needed, and > + the index of the doorbell bit with the channel when > + compatible is "arm,mhu-doorbell" > - interrupts: Contains the interrupt information corresponding to > - each of the 3 links of MHU. > + each of the 3 physical channels of MHU namely low > + priority non-secure, high priority non-secure and > + secure channels. > > Example: > > > +1. Controller which doesn't support doorbells > + > mhu: mailbox@2b1f { > #mbox-cells = <1>; > compatible = "arm,mhu", "arm,primecell"; > @@ -41,3 +62,22 @@ used by Linux running NS. > reg = <0 0x2e00 0x4000>; > mboxes = < 1>; /* HP-NonSecure */ > }; > + > +2. Controller which supports doorbells > + > + mhu: mailbox@2b1f { > + #mbox-cells = <2>; > + compatible = "arm,mhu-doorbell", "arm,primecell"; > + reg = <0 0x2b1f 0x1000>; > + interrupts = <0 36 4>, /* LP-NonSecure */ > +<0 35 4>, /* HP-NonSecure */ > +<0 37 4>; /* Secure */ > + clocks = < 0 2 1>; > + clock-names = "apb_pclk"; > + }; > + > + mhu_client: scb@2e00 { > + compatible = "arm,scpi"; > + reg = <0 0x2e00 0x200>; > + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ > + }; > Every MHU controller can by driven as "arm,mhu-doorbell" or "arm,mhu" equally fine. So you are basically smuggling a s/w feature into DT.
[PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
The ARM MHU has mechanism to assert interrupt signals to facilitate inter-processor message based communication. It drives the signal using a 32-bit register, with all 32-bits logically ORed together. It also enables software to set, clear and check the status of each of the bits of this register independently. Each bit of the register can be associated with a type of event that can contribute to raising the interrupt thereby allowing it to be used as independent doorbells. Since the first version of this binding can't support doorbells, this patch extends the existing binding to support them. Cc: Alexey KlimovCc: Jassi Brar Cc: Rob Herring Cc: devicet...@vger.kernel.org Signed-off-by: Sudeep Holla --- .../devicetree/bindings/mailbox/arm-mhu.txt| 46 -- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt index 4971f03f0b33..bd9a3a267caf 100644 --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read the data. The last channel is specified to be a 'Secure' resource, hence can't be used by Linux running NS. +The MHU drives the interrupt signal using a 32-bit register, with all +32-bits logically ORed together. It provides a set of registers to +enable software to set, clear and check the status of each of the bits +of this register independently. The use of 32 bits per interrupt line +enables software to provide more information about the source of the +interrupt. For example, each bit of the register can be associated with +a type of event that can contribute to raising the interrupt. Each of +the 32-bits can be used as "doorbell" to alert the remote processor. + Mailbox Device Node: Required properties: -- compatible: Shall be "arm,mhu" & "arm,primecell" +- compatible: Shall be "arm,primecell" and one of the below: + "arm,mhu" - if the controller doesn't support + doorbell model + "arm,mhu-doorbell" - if the controller supports + doorbell model - reg: Contains the mailbox register address range (base address and length) -- #mbox-cells Shall be 1 - the index of the channel needed. +- #mbox-cells Shall be 1 - the index of the channel needed when + compatible is "arm,mhu" + Shall be 2 - the index of the channel needed, and + the index of the doorbell bit with the channel when + compatible is "arm,mhu-doorbell" - interrupts: Contains the interrupt information corresponding to - each of the 3 links of MHU. + each of the 3 physical channels of MHU namely low + priority non-secure, high priority non-secure and + secure channels. Example: +1. Controller which doesn't support doorbells + mhu: mailbox@2b1f { #mbox-cells = <1>; compatible = "arm,mhu", "arm,primecell"; @@ -41,3 +62,22 @@ used by Linux running NS. reg = <0 0x2e00 0x4000>; mboxes = < 1>; /* HP-NonSecure */ }; + +2. Controller which supports doorbells + + mhu: mailbox@2b1f { + #mbox-cells = <2>; + compatible = "arm,mhu-doorbell", "arm,primecell"; + reg = <0 0x2b1f 0x1000>; + interrupts = <0 36 4>, /* LP-NonSecure */ +<0 35 4>, /* HP-NonSecure */ +<0 37 4>; /* Secure */ + clocks = < 0 2 1>; + clock-names = "apb_pclk"; + }; + + mhu_client: scb@2e00 { + compatible = "arm,scpi"; + reg = <0 0x2e00 0x200>; + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ + }; -- 2.7.4
[PATCH v2 2/6] Documentation: devicetree: add bindings to support ARM MHU doorbells
The ARM MHU has mechanism to assert interrupt signals to facilitate inter-processor message based communication. It drives the signal using a 32-bit register, with all 32-bits logically ORed together. It also enables software to set, clear and check the status of each of the bits of this register independently. Each bit of the register can be associated with a type of event that can contribute to raising the interrupt thereby allowing it to be used as independent doorbells. Since the first version of this binding can't support doorbells, this patch extends the existing binding to support them. Cc: Alexey Klimov Cc: Jassi Brar Cc: Rob Herring Cc: devicet...@vger.kernel.org Signed-off-by: Sudeep Holla --- .../devicetree/bindings/mailbox/arm-mhu.txt| 46 -- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt index 4971f03f0b33..bd9a3a267caf 100644 --- a/Documentation/devicetree/bindings/mailbox/arm-mhu.txt +++ b/Documentation/devicetree/bindings/mailbox/arm-mhu.txt @@ -10,21 +10,42 @@ STAT register and the remote clears it after having read the data. The last channel is specified to be a 'Secure' resource, hence can't be used by Linux running NS. +The MHU drives the interrupt signal using a 32-bit register, with all +32-bits logically ORed together. It provides a set of registers to +enable software to set, clear and check the status of each of the bits +of this register independently. The use of 32 bits per interrupt line +enables software to provide more information about the source of the +interrupt. For example, each bit of the register can be associated with +a type of event that can contribute to raising the interrupt. Each of +the 32-bits can be used as "doorbell" to alert the remote processor. + Mailbox Device Node: Required properties: -- compatible: Shall be "arm,mhu" & "arm,primecell" +- compatible: Shall be "arm,primecell" and one of the below: + "arm,mhu" - if the controller doesn't support + doorbell model + "arm,mhu-doorbell" - if the controller supports + doorbell model - reg: Contains the mailbox register address range (base address and length) -- #mbox-cells Shall be 1 - the index of the channel needed. +- #mbox-cells Shall be 1 - the index of the channel needed when + compatible is "arm,mhu" + Shall be 2 - the index of the channel needed, and + the index of the doorbell bit with the channel when + compatible is "arm,mhu-doorbell" - interrupts: Contains the interrupt information corresponding to - each of the 3 links of MHU. + each of the 3 physical channels of MHU namely low + priority non-secure, high priority non-secure and + secure channels. Example: +1. Controller which doesn't support doorbells + mhu: mailbox@2b1f { #mbox-cells = <1>; compatible = "arm,mhu", "arm,primecell"; @@ -41,3 +62,22 @@ used by Linux running NS. reg = <0 0x2e00 0x4000>; mboxes = < 1>; /* HP-NonSecure */ }; + +2. Controller which supports doorbells + + mhu: mailbox@2b1f { + #mbox-cells = <2>; + compatible = "arm,mhu-doorbell", "arm,primecell"; + reg = <0 0x2b1f 0x1000>; + interrupts = <0 36 4>, /* LP-NonSecure */ +<0 35 4>, /* HP-NonSecure */ +<0 37 4>; /* Secure */ + clocks = < 0 2 1>; + clock-names = "apb_pclk"; + }; + + mhu_client: scb@2e00 { + compatible = "arm,scpi"; + reg = <0 0x2e00 0x200>; + mboxes = < 1 4>; /* HP-NonSecure 5th doorbell bit */ + }; -- 2.7.4