On Wed, May 3, 2017 at 2:51 PM, Sudeep Holla <sudeep.ho...@arm.com> wrote: > > > On 03/05/17 04:17, Jassi Brar wrote: >> Hi Sudeep, >> >> On Tue, May 2, 2017 at 7:25 PM, Sudeep Holla <sudeep.ho...@arm.com> wrote: >>> Hi Jassi, >>> >>> This series adds subchannel support to ARM MHU mailbox controller >>> driver. Since SCPI never used second slot, we were able to use the >>> existing driver as is. However, that's changing soon and the new >>> SCMI protocol under development needs subchannel support. If you >>> recall when you initially added this driver, I was pushing for some >>> of these changes like threaded irq. This patch series adds support >>> for the subchannels on ARM MHU controllers. >>> >> There are really no "sub-channels" in the ARM MHU controller. There >> are exactly three channels that work on 32bit registers. The SET/CLEAR >> registers are there to prevent races between local and remote >> firmware, and not to emulate virtual channels operating on single >> bits. Please remember all 32-bits work together to generate one >> signal. >> > > If you check 3.4.4 Message Handling Unit (MHU) of Juno TRM [1], > > "..the MHU drives the signal using a 32-bit register, with all 32 bits > logically ORed together. The MHU 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 for each 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." > > So yes, they generate one signal, but that doesn't mean anything. > That means a lot. That means a MHU signal/message is 32bits, not single bit.
> We > have even PMU interrupts tied to single SPI on some SoC. Since the > design of MHU clearly indicates that each bit can be used independently > for different event, for all practical purpose, it can be treated as > different channel. > Please don't mess with controller driver to support your usecase, which is already well supported. >> You arrived at the "sub-channel" idea only because your protocol uses >> 1-bit messages. > > May be. It now uses BIT 0 for one channel and BIT 1 for another on the > same physical channel. How do you propose it support that then ? > There is no "sub-channel", but only physical channel. You are led to believe each bit represents one channel only because your protocol uses (1<<N) type signals. > We have > multiple protocols with the same remote, so this is just used as a > doorbell bit and not carrier of any message. > Doorbell is a single bit message in mailbox framework :) >> This patchset seems rather regressive - reduce from >> 2^32 possible signals to mere 32, by bloating the MHU driver. >> > > I don't quite get this. There are only 3 signals as you mentioned above. > Yes there are 2^32 possible values for the register, but how can that be > used ? > _Your_ protocol don't use more than 32 values, that doesn't mean other protocols don't either.