| Message ID | ccbb0641fa123021249c7a51cbe2dcd975c41f39.1504677110.git.baolin.wang@spreadtrum.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 06, 2017 at 02:10:43PM +0800, Baolin Wang wrote: > +- hwlocks: Reference to a phandle of a hwlock provider node. > +- hwlock-names: Reference to hwlock name strings defined in the same order > + as the hwlocks. What are these hwlocks protecting, and what names are expected? > +Optional properties: > +- sprd,hw-channels: Specify the hardware channel number and mapped address > + for hardware channel accessing. What do these mean and how are the numbers and how will the binding be interpreted?
Hi Mark, On 6 September 2017 at 22:59, Mark Brown <broonie@kernel.org> wrote: > On Wed, Sep 06, 2017 at 02:10:43PM +0800, Baolin Wang wrote: > >> +- hwlocks: Reference to a phandle of a hwlock provider node. >> +- hwlock-names: Reference to hwlock name strings defined in the same order >> + as the hwlocks. > > What are these hwlocks protecting, and what names are expected? I made one explanation in above sentence, I assume it is not clear. Since we have multi-subsystems will use ADI to access analog chip, when one system is reading/writing data by ADI, which should be under one hardware spinlock protection to prevent other systems from reading/writing data by ADI at the same time, or two parallel routine of setting ADI registers will get incorrect results. The hwspinlock name should be "adi", and I will make it clear in next version. > >> +Optional properties: >> +- sprd,hw-channels: Specify the hardware channel number and mapped address >> + for hardware channel accessing. > > What do these mean and how are the numbers and how will the binding be > interpreted? I also gave one explanation in above sentence, is it not clear? I try again. ADI controller has 50 channels including 2 software read/write channels and 48 hardware channels to access analog chip. For 2 software read/write channels, which means we should set ADI registers to access analog chip. But For hardware channels, we can just mapped one analog chip address to one hardware channel, then user can access analog chip by hardware channel without setting ADI registers. For this "sprd,hw-channels" property, the first value specifies the channel id, and the second value specifies the address which is mapped into analog chip space.
On Thu, Sep 07, 2017 at 11:29:05AM +0800, Baolin Wang wrote: > On 6 September 2017 at 22:59, Mark Brown <broonie@kernel.org> wrote: > > On Wed, Sep 06, 2017 at 02:10:43PM +0800, Baolin Wang wrote: > >> +- hwlocks: Reference to a phandle of a hwlock provider node. > >> +- hwlock-names: Reference to hwlock name strings defined in the same order > >> + as the hwlocks. > > What are these hwlocks protecting, and what names are expected? > I made one explanation in above sentence, I assume it is not clear. > Since we have multi-subsystems will use ADI to access analog chip, > when one system is reading/writing data by ADI, which should be under > one hardware spinlock protection to prevent other systems from > reading/writing data by ADI at the same time, or two parallel routine > of setting ADI registers will get incorrect results. > The hwspinlock name should be "adi", and I will make it clear in next version. So there's other drivers that might also be accessing this IP block? > >> +Optional properties: > >> +- sprd,hw-channels: Specify the hardware channel number and mapped address > >> + for hardware channel accessing. > > What do these mean and how are the numbers and how will the binding be > > interpreted? > I also gave one explanation in above sentence, is it not clear? I try again. It says what they are, it doesn't say for example what a hardware channel is or how those numbers map onto the actual hardware. > ADI controller has 50 channels including 2 software read/write > channels and 48 hardware channels to access analog chip. For 2 > software read/write channels, which means we should set ADI registers > to access analog chip. But For hardware channels, we can just mapped > one analog chip address to one hardware channel, then user can access > analog chip by hardware channel without setting ADI registers. > For this "sprd,hw-channels" property, the first value specifies the > channel id, and the second value specifies the address which is mapped > into analog chip space. So does this driver control all the channels or are there other drivers (or hardware components) that control some of the other channels?
On 7 September 2017 at 17:54, Mark Brown <broonie@kernel.org> wrote: > On Thu, Sep 07, 2017 at 11:29:05AM +0800, Baolin Wang wrote: >> On 6 September 2017 at 22:59, Mark Brown <broonie@kernel.org> wrote: >> > On Wed, Sep 06, 2017 at 02:10:43PM +0800, Baolin Wang wrote: > >> >> +- hwlocks: Reference to a phandle of a hwlock provider node. >> >> +- hwlock-names: Reference to hwlock name strings defined in the same order >> >> + as the hwlocks. > >> > What are these hwlocks protecting, and what names are expected? > >> I made one explanation in above sentence, I assume it is not clear. >> Since we have multi-subsystems will use ADI to access analog chip, >> when one system is reading/writing data by ADI, which should be under >> one hardware spinlock protection to prevent other systems from >> reading/writing data by ADI at the same time, or two parallel routine >> of setting ADI registers will get incorrect results. > >> The hwspinlock name should be "adi", and I will make it clear in next version. > > So there's other drivers that might also be accessing this IP block? Yes. Other drivers (like regulator, RTC or charger ... ) can access analog chip (like PMIC) by ADI controller. But the hardware spinlock is used to synchronize between the multiple subsystems, since we only have one ADI controller. > >> >> +Optional properties: >> >> +- sprd,hw-channels: Specify the hardware channel number and mapped address >> >> + for hardware channel accessing. > >> > What do these mean and how are the numbers and how will the binding be >> > interpreted? > >> I also gave one explanation in above sentence, is it not clear? I try again. > > It says what they are, it doesn't say for example what a hardware > channel is or how those numbers map onto the actual hardware. OK. I will add more documentation to explain that. > >> ADI controller has 50 channels including 2 software read/write >> channels and 48 hardware channels to access analog chip. For 2 >> software read/write channels, which means we should set ADI registers >> to access analog chip. But For hardware channels, we can just mapped >> one analog chip address to one hardware channel, then user can access >> analog chip by hardware channel without setting ADI registers. > >> For this "sprd,hw-channels" property, the first value specifies the >> channel id, and the second value specifies the address which is mapped >> into analog chip space. > > So does this driver control all the channels or are there other drivers > (or hardware components) that control some of the other channels? The ADI driver only controls 2 software channels (read/write), and other hardware channels can be controlled by hardware components if we set the hardware config.
On Thu, Sep 07, 2017 at 07:03:18PM +0800, Baolin Wang wrote: > On 7 September 2017 at 17:54, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Sep 07, 2017 at 11:29:05AM +0800, Baolin Wang wrote: > >> The hwspinlock name should be "adi", and I will make it clear in next version. > > So there's other drivers that might also be accessing this IP block? > Yes. Other drivers (like regulator, RTC or charger ... ) can access > analog chip (like PMIC) by ADI controller. But the hardware spinlock > is used to synchronize between the multiple subsystems, since we only > have one ADI controller. If it were other drivers then the kernel should already be doing that but... > > So does this driver control all the channels or are there other drivers > > (or hardware components) that control some of the other channels? > The ADI driver only controls 2 software channels (read/write), and > other hardware channels can be controlled by hardware components if we > set the hardware config. ...if it can be configured to allow other things to use it independently then this is fine, just needs a bit more documentation so someone can understand how the bindings and hardware match up.
On 7 September 2017 at 19:44, Mark Brown <broonie@kernel.org> wrote: > On Thu, Sep 07, 2017 at 07:03:18PM +0800, Baolin Wang wrote: >> On 7 September 2017 at 17:54, Mark Brown <broonie@kernel.org> wrote: >> > On Thu, Sep 07, 2017 at 11:29:05AM +0800, Baolin Wang wrote: > >> >> The hwspinlock name should be "adi", and I will make it clear in next version. > >> > So there's other drivers that might also be accessing this IP block? > >> Yes. Other drivers (like regulator, RTC or charger ... ) can access >> analog chip (like PMIC) by ADI controller. But the hardware spinlock >> is used to synchronize between the multiple subsystems, since we only >> have one ADI controller. > > If it were other drivers then the kernel should already be doing that > but... Not only kernel drivers, but also other systems' drivers will access ADI too. So here we need one hardware spinlock to protect, I will add more documentation. > >> > So does this driver control all the channels or are there other drivers >> > (or hardware components) that control some of the other channels? > >> The ADI driver only controls 2 software channels (read/write), and >> other hardware channels can be controlled by hardware components if we >> set the hardware config. > > ...if it can be configured to allow other things to use it independently > then this is fine, just needs a bit more documentation so someone can > understand how the bindings and hardware match up. OK. Thanks for your comments.
diff --git a/Documentation/devicetree/bindings/spi/spi-sprd-adi.txt b/Documentation/devicetree/bindings/spi/spi-sprd-adi.txt new file mode 100644 index 0000000..11c5259 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/spi-sprd-adi.txt @@ -0,0 +1,44 @@ +Spreadtrum ADI controller based on SPI framework + +ADI is the abbreviation of Anolog-Digital interface, which is used to access +analog chip (such as PMIC) from digital chip. ADI controller follows the SPI +framework for its hardware implementation is alike to SPI bus and its timing +is compatile to SPI timing. + +ADI controller has 50 channels including 2 software read/write channels and +48 hardware channels to access analog chip. For hardware channels, we introduce +one property named "sprd,hw-channels" to configure hardware channels, the first +value specifies the channel id, and the second value specifies the address which +is mapped into analog chip space. + +Since we have multi-subsystems will use ADI to access analog chip, then we need +one hardware spinlock to synchronize between the multiple subsystems. + +Required properties: +- compatible: Should be "sprd,sc9860-adi". +- reg: Offset and length of ADI-SPI controller register space. +- hwlocks: Reference to a phandle of a hwlock provider node. +- hwlock-names: Reference to hwlock name strings defined in the same order + as the hwlocks. +- #address-cells: Number of cells required to define a chip select address + on the ADI-SPI bus. Should be set to 1. +- #size-cells: Size of cells required to define a chip select address size + on the ADI-SPI bus. Should be set to 0. + +Optional properties: +- sprd,hw-channels: Specify the hardware channel number and mapped address + for hardware channel accessing. + +SPI slave nodes must be children of the SPI controller node and can contain +properties described in Documentation/devicetree/bindings/spi/spi-bus.txt. + +Example: + adi_bus: spi@40030000 { + compatible = "sprd,sc9860-adi"; + reg = <0 0x40030000 0 0x10000>; + hwlocks = <&hwlock1 0>; + hwlock-names = "adi"; + #address-cells = <1>; + #size-cells = <0>; + sprd,hw-channels = <30 0x8c20>; + };
This patch adds the binding documentation for Spreadtrum ADI controller device. Signed-off-by: Baolin Wang <baolin.wang@spreadtrum.com> --- .../devicetree/bindings/spi/spi-sprd-adi.txt | 44 ++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 Documentation/devicetree/bindings/spi/spi-sprd-adi.txt