serial: 8250_dw: Add DMA support for non-ACPI platforms

The dma pointer under struct uart_8250_port is currently left unassigned for non-ACPI platforms. It should be pointing to the dma member in struct dw8250_data like how it was done for ACPI, so the core 8250 code will try to request for DMA when registering the port

If DMA is not enabled in device tree, request DMA will fail and the driver will fall back to PIO

Signed-off-by Ray Jui <[email protected]>

Reviewed-by JD (Jiandong) Zheng <[email protected]>

Reviewed-by Scott Branden <[email protected]>

Tested-by Scott Branden <[email protected]>

The PL022 SPI driver maps the DMA RX buffer before the DMA TX buffer. In most cases, the sequence of the mapping does not matter. But in cases where TX and RX happen to use the same buffer, e.g., spidev, it causes the cached TX data not written to memory, because the same memory has been marked invalid when dma_map_sg on the RX buffer is called

Quoted 5 lines · Ray Jui

The PL022 SPI driver maps the DMA RX buffer before the DMA TX buffer. In most cases, the sequence of the mapping does not matter. But in cases where TX and RX happen to use the same buffer, e.g., spidev, it causes the cached TX data not written to memory, because the same memory has been marked invalid when dma_map_sg on the RX buffer is called

This seems like it is a bug in spidev, using the same buffer simultaneously for both directions isn't something I'd think would be expected to work reliably unless it was explicitly mapped as bidirectional.

Thanks for the reply. It looks like you are also the maintainer of spidev. In this case, could you please help to confirm that you expect spidev to use separate buffers for TX and RX? If so, I can go ahead and make the change in spidev.

Quoted 4 lines · Ray Jui

Thanks for the reply. It looks like you are also the maintainer of spidev. In this case, could you please help to confirm that you expect spidev to use separate buffers for TX and RX? If so, I can go ahead and make the change in spidev.

Yes, that would be my expectation for maximum robustness (or if it is going to use one buffer it explicitly maps it for mixed use but I'd expect that to be asking for trouble).

Quoted 4 lines · Ray Jui

Thanks for the reply. It looks like you are also the maintainer of spidev. In this case, could you please help to confirm that you expect spidev to use separate buffers for TX and RX? If so, I can go ahead and make the change in spidev.

Yes, that would be my expectation for maximum robustness (or if it is going to use one buffer it explicitly maps it for mixed use but I'd expect that to be asking for trouble).

By using separate TX and RX bounce buffers, we avoid potential cache flush and invalidation sequence issue that may be encountered when a single bounce buffer is shared between TX and RX

Quoted 3 lines · Ray Jui

By using separate TX and RX bounce buffers, we avoid potential cache flush and invalidation sequence issue that may be encountered when a single bounce buffer is shared between TX and RX

Applied, thanks.

When mapped RX DMA entries are unmapped in an error condition when DMA is firstly configured in the driver, the number of TX DMA entries was passed in, which is incorrect

Quoted 3 lines · Ray Jui

When mapped RX DMA entries are unmapped in an error condition when DMA is firstly configured in the driver, the number of TX DMA entries was passed in, which is incorrect

Applied, thanks.

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Quoted 3 lines · Ray Jui

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Well, we do have -EPROBE_DEFER these days to handle these kinds of dependencies so we no longer have to these kinds of manual init reordering tricks.

Quoted 5 lines · Lars-Peter Clausen

Quoted 3 lines · Ray Jui

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Quoted 3 lines

Well, we do have -EPROBE_DEFER these days to handle these kinds of dependencies so we no longer have to these kinds of manual init reordering tricks.

How ould that work?

Consider for example SPI and dmanegine. SPI driver got probed, then to start a transaction requested a channel... while dmaengine driver is still getting probed/not probed yet. So SPI driver didnt get a channel.

Quoted 9 lines · Vinod Koul

Quoted 8 lines · Lars-Peter Clausen

Quoted 3 lines · Ray Jui

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Well, we do have -EPROBE_DEFER these days to handle these kinds of dependencies so we no longer have to these kinds of manual init reordering tricks.

Quoted 8 lines

How ould that work?

Consider for example SPI and dmanegine. SPI driver got probed, then to start
a transaction requested a channel... while dmaengine driver is still getting
probed/not probed yet. So SPI driver didnt get a channel.

Ideally the SPI driver requests the channel in probe function and if the DMA controller is not yet probed returns EPROBE_DEFER. If the SPI driver requests the channel in the transfer handler it needs to deal with being able to fall back to non DMA transfers anyway so this shouldn't be a problem.

But in any case fiddling around with the init sequences is just a quick hack and might makes the problem less likely to appear in some cases, but there is no guarantee that it works. And I think the proper solution at the moment is to use probe deferral.

Other subsystems have seen patches which moved drivers from using subsys_initcall to device_initcall/module_..._driver/ with the reasoning that this is no longer necessary because of EPROBE_DEFER. So I don't think we should be doing the exact opposite in DMA framework. Also if we'd apply this patch it won't take to long until somebody suggest going back to module_platform_driver() instead of subsys_initcall.

- Lars

Quoted 19 lines · Lars-Peter Clausen

Quoted 9 lines · Vinod Koul

Quoted 8 lines · Lars-Peter Clausen

Quoted 3 lines · Ray Jui

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Well, we do have -EPROBE_DEFER these days to handle these kinds of dependencies so we no longer have to these kinds of manual init reordering tricks.

Quoted 8 lines

How ould that work?

Consider for example SPI and dmanegine. SPI driver got probed, then to start
a transaction requested a channel... while dmaengine driver is still getting
probed/not probed yet. So SPI driver didnt get a channel.

Quoted 5 lines

Ideally the SPI driver requests the channel in probe function and if the DMA controller is not yet probed returns EPROBE_DEFER. If the SPI driver requests the channel in the transfer handler it needs to deal with being able to fall back to non DMA transfers anyway so this shouldn't be a problem.

So in the case of the spi-pl022 driver. It requests the channel in probe function. And obviously DMA is not mandatory, so when the channel request fails the probe won't fail and instead it falls back to PIO. In this case, can you recommend a different way to solve this problem without having the DMA driver probed earlier than its slaves?

Quoted 6 lines

But in any case fiddling around with the init sequences is just a quick hack and might makes the problem less likely to appear in some cases, but there is no guarantee that it works. And I think the proper solution at the moment is to use probe deferral.

I think it makes sense to have the DMA driver, as one of the core components in various SoCs that a lot of peripheral drivers depend on, to be registered at the level of subsys_init or somewhere close. We are not changing this just to get SPI to work. We are changing this because we think DMA should be ready before a lot of its slaves, which are typically done at device_initcall.

I have no problem relying on EPROBE_DEFER for this, provided that it works. The issue is, like I mentioned above, for a lot of slave devices DMA is not mandatory, when DMA fails at probe they would fall back to PIO and never use DMA. Another disadvantage I see with EPROBE_DEFER is delayed boot time.

Quoted 8 lines

Other subsystems have seen patches which moved drivers from using subsys_initcall to device_initcall/module_..._driver/ with the reasoning that this is no longer necessary because of EPROBE_DEFER. So I don't think we should be doing the exact opposite in DMA framework. Also if we'd apply this patch it won't take to long until somebody suggest going back to module_platform_driver() instead of subsys_initcall.

Quoted 1 line

- Lars

There are currently 12 DMA drivers under drivers/dma registering themselves at subsys_init. I don't see why pl330 cannot do the same. Is there any concern that it may not work for some other SoCs when it's done at subsys_init? So far I cannot think of any. The only dependency of pl330 is the ARM apb_pclk, required during AMBA bus probe. But that's usually ready before subsys_init.

Quoted 25 lines · Ray Jui

Quoted 19 lines · Lars-Peter Clausen

Quoted 9 lines · Vinod Koul

Quoted 8 lines · Lars-Peter Clausen

Quoted 3 lines · Ray Jui

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Well, we do have -EPROBE_DEFER these days to handle these kinds of dependencies so we no longer have to these kinds of manual init reordering tricks.

Quoted 8 lines

How ould that work?

Consider for example SPI and dmanegine. SPI driver got probed, then to start
a transaction requested a channel... while dmaengine driver is still getting
probed/not probed yet. So SPI driver didnt get a channel.

Quoted 5 lines

Ideally the SPI driver requests the channel in probe function and if the DMA controller is not yet probed returns EPROBE_DEFER. If the SPI driver requests the channel in the transfer handler it needs to deal with being able to fall back to non DMA transfers anyway so this shouldn't be a problem.

Quoted 7 lines

So in the case of the spi-pl022 driver. It requests the channel in probe function. And obviously DMA is not mandatory, so when the channel request fails the probe won't fail and instead it falls back to PIO. In this case,
can you recommend a different way to solve this problem without having the
DMA driver probed earlier than its slaves?

dma_request_slave_channel() has the problem that we can't differentiate between no channel provided and channel provided but the dma driver hasn't probed yet. The function will return NULL in both cases. But Stephen Warren added dma_request_slave_channel_reason() a while ago to solve this problem. This function returns a ERR_PTR. If it returns ERR_PTR(-EPROBE_DEFER) it means that a channel has been provided but the DMA driver hasn't probed yet. In this case the SPI driver should return -EPROBE_DEFER to try again later. If the function returns a different error code that means that it was not possible to get the DMA channel and it should fall back to PIO.

Quoted 6 lines

Quoted 6 lines

But in any case fiddling around with the init sequences is just a quick hack and might makes the problem less likely to appear in some cases, but there is no guarantee that it works. And I think the proper solution at the moment is to use probe deferral.

Quoted 9 lines

I think it makes sense to have the DMA driver, as one of the core components
in various SoCs that a lot of peripheral drivers depend on, to be registered
at the level of subsys_init or somewhere close. We are not changing this just to get SPI to work. We are changing this because we think DMA should be
ready before a lot of its slaves, which are typically done at device_initcall.

But if the DMA driver for example depends on a clock driver do you put the clock driver at a even earlier init level? The problem with using init levels for solving this problem is that there is only a small amount of init levels available and representing the dependency chains is neither possible with it nor were init level ever intended for solving this. EPROBE_DEFER on the other hand is.

Quoted 9 lines

I have no problem relying on EPROBE_DEFER for this, provided that it works.
The issue is, like I mentioned above, for a lot of slave devices DMA is not
mandatory, when DMA fails at probe they would fall back to PIO and never use
DMA. Another disadvantage I see with EPROBE_DEFER is delayed boot time.

Yea, the EPROBE_DEFER implementation is not ideal, but that is a problem that should be solved rather than working around it. I think there are patches somewhere for example that build a device dependency graph from the phandles in the devicetree and than probe devices in the correct order to reduce the number of times probe deferral is necessary.

Quoted 9 lines

Quoted 9 lines

Other subsystems have seen patches which moved drivers from using subsys_initcall to device_initcall/module_..._driver/ with the reasoning that this is no longer necessary because of EPROBE_DEFER. So I don't think we should be doing the exact opposite in DMA framework. Also if we'd apply this patch it won't take to long until somebody suggest going back to module_platform_driver() instead of subsys_initcall.

- Lars

Quoted 9 lines

There are currently 12 DMA drivers under drivers/dma registering themselves
at subsys_init. I don't see why pl330 cannot do the same. Is there any concern that it may not work for some other SoCs when it's done at subsys_init? So far I cannot think of any. The only dependency of pl330 is
the ARM apb_pclk, required during AMBA bus probe. But that's usually ready
before subsys_init.

Those other drivers should be converted to device_initcall rather than converting the PL330 driver to subsys_init. Using subsys_init for device drivers is a hack which was used to try to solve ordering problems. But it doesn't work that great, especially if you have more than two devices in your dependency chain. The solution that people have come up with to solve this problem in a better way is probe deferral by the means of -EPROBE_DEFER.

- Lars

Quoted 25 lines · Ray Jui

Quoted 19 lines · Lars-Peter Clausen

Quoted 9 lines · Vinod Koul

Quoted 8 lines · Lars-Peter Clausen

Quoted 3 lines · Ray Jui

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Well, we do have -EPROBE_DEFER these days to handle these kinds of dependencies so we no longer have to these kinds of manual init reordering tricks.

Quoted 8 lines

How ould that work?

Consider for example SPI and dmanegine. SPI driver got probed, then to start
a transaction requested a channel... while dmaengine driver is still getting
probed/not probed yet. So SPI driver didnt get a channel.

Quoted 5 lines

Ideally the SPI driver requests the channel in probe function and if the DMA controller is not yet probed returns EPROBE_DEFER. If the SPI driver requests the channel in the transfer handler it needs to deal with being able to fall back to non DMA transfers anyway so this shouldn't be a problem.

Quoted 5 lines

So in the case of the spi-pl022 driver. It requests the channel in probe function. And obviously DMA is not mandatory, so when the channel request fails the probe won't fail and instead it falls back to PIO. In this case, can you recommend a different way to solve this problem without having the DMA driver probed earlier than its slaves?

dma_request_slave_channel() has the problem that we can't differentiate between no channel provided and channel provided but the dma driver hasn't probed yet. The function will return NULL in both cases. But Stephen Warren added dma_request_slave_channel_reason() a while ago to solve this problem. This function returns a ERR_PTR. If it returns ERR_PTR(-EPROBE_DEFER) it means that a channel has been provided but the DMA driver hasn't probed yet. In this case the SPI driver should return -EPROBE_DEFER to try again later. If the function returns a different error code that means that it was not possible to get the DMA channel and it should fall back to PIO.

Quoted 7 lines

Quoted 7 lines

But in any case fiddling around with the init sequences is just a quick hack and might makes the problem less likely to appear in some cases, but there is no guarantee that it works. And I think the proper solution at the moment is to use probe deferral.

Quoted 5 lines

I think it makes sense to have the DMA driver, as one of the core components in various SoCs that a lot of peripheral drivers depend on, to be registered at the level of subsys_init or somewhere close. We are not changing this just to get SPI to work. We are changing this because we think DMA should be ready before a lot of its slaves, which are typically done at device_initcall.

But if the DMA driver for example depends on a clock driver do you put the clock driver at a even earlier init level? The problem with using init levels for solving this problem is that there is only a small amount of init levels available and representing the dependency chains is neither possible with it nor were init level ever intended for solving this. EPROBE_DEFER on the other hand is.

Quoted 6 lines

I have no problem relying on EPROBE_DEFER for this, provided that it works. The issue is, like I mentioned above, for a lot of slave devices DMA is not mandatory, when DMA fails at probe they would fall back to PIO and never use DMA. Another disadvantage I see with EPROBE_DEFER is delayed boot time.

Yea, the EPROBE_DEFER implementation is not ideal, but that is a problem that should be solved rather than working around it. I think there are patches somewhere for example that build a device dependency graph from the phandles in the devicetree and than probe devices in the correct order to reduce the number of times probe deferral is necessary.

Quoted 8 lines

Quoted 8 lines

Other subsystems have seen patches which moved drivers from using subsys_initcall to device_initcall/module_..._driver/ with the reasoning that this is no longer necessary because of EPROBE_DEFER. So I don't think we should be doing the exact opposite in DMA framework. Also if we'd apply this patch it won't take to long until somebody suggest going back to module_platform_driver() instead of subsys_initcall.

Quoted 7 lines

Quoted 1 line

- Lars

There are currently 12 DMA drivers under drivers/dma registering themselves at subsys_init. I don't see why pl330 cannot do the same. Is there any concern that it may not work for some other SoCs when it's done at subsys_init? So far I cannot think of any. The only dependency of pl330 is the ARM apb_pclk, required during AMBA bus probe. But that's usually ready before subsys_init.

Those other drivers should be converted to device_initcall rather than converting the PL330 driver to subsys_init. Using subsys_init for device drivers is a hack which was used to try to solve ordering problems. But it doesn't work that great, especially if you have more than two devices in your dependency chain. The solution that people have come up with to solve this problem in a better way is probe deferral by the means of -EPROBE_DEFER.

- Lars

Quoted 33 lines · Lars-Peter Clausen

Quoted 25 lines · Ray Jui

Quoted 19 lines · Lars-Peter Clausen

Quoted 9 lines · Vinod Koul

Quoted 8 lines · Lars-Peter Clausen

Quoted 3 lines · Ray Jui

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Well, we do have -EPROBE_DEFER these days to handle these kinds of dependencies so we no longer have to these kinds of manual init reordering tricks.

Quoted 8 lines

How ould that work?

Consider for example SPI and dmanegine. SPI driver got probed, then to start
a transaction requested a channel... while dmaengine driver is still getting
probed/not probed yet. So SPI driver didnt get a channel.

Quoted 5 lines

Ideally the SPI driver requests the channel in probe function and if the DMA controller is not yet probed returns EPROBE_DEFER. If the SPI driver requests the channel in the transfer handler it needs to deal with being able to fall back to non DMA transfers anyway so this shouldn't be a problem.

Quoted 5 lines

So in the case of the spi-pl022 driver. It requests the channel in probe function. And obviously DMA is not mandatory, so when the channel request fails the probe won't fail and instead it falls back to PIO. In this case, can you recommend a different way to solve this problem without having the DMA driver probed earlier than its slaves?

Quoted 11 lines

dma_request_slave_channel() has the problem that we can't differentiate between no channel provided and channel provided but the dma driver hasn't probed yet. The function will return NULL in both cases. But Stephen Warren added dma_request_slave_channel_reason() a while ago to solve this problem. This function returns a ERR_PTR. If it returns ERR_PTR(-EPROBE_DEFER) it means that a channel has been provided but the DMA driver hasn't probed yet. In this case the SPI driver should return -EPROBE_DEFER to try again later. If the function returns a different error code that means that it was not possible to get the DMA channel and it should fall back to PIO.

So when should the SPI here check, if dmaengine is available. The client doesn't grab channel in its probe, so the client cannot return -EPROBE_DEFER.

Quoted 6 lines · Vinod Koul

Quoted 5 lines · Lars-Peter Clausen

Quoted 3 lines · Ray Jui

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Quoted 9 lines

Quoted 3 lines

Well, we do have -EPROBE_DEFER these days to handle these kinds of dependencies so we no longer have to these kinds of manual init reordering tricks.

How ould that work?

Consider for example SPI and dmanegine. SPI driver got probed, then to start a transaction requested a channel... while dmaengine driver is still getting probed/not probed yet. So SPI driver didnt get a channel.

Ideally the SPI driver requests the channel in probe function and if the DMA controller is not yet probed returns EPROBE_DEFER. If the SPI driver requests the channel in the transfer handler it needs to deal with being able to fall back to non DMA transfers anyway so this shouldn't be a problem.

But in any case fiddling around with the init sequences is just a quick hack and might makes the problem less likely to appear in some cases, but there is no guarantee that it works. And I think the proper solution at the moment is to use probe deferral.

Other subsystems have seen patches which moved drivers from using subsys_initcall to device_initcall/module_..._driver/ with the reasoning that this is no longer necessary because of EPROBE_DEFER. So I don't think we should be doing the exact opposite in DMA framework. Also if we'd apply this patch it won't take to long until somebody suggest going back to module_platform_driver() instead of subsys_initcall.

As part of subsystem that many slave drivers depend on, it's more appropriate for the pl330 DMA driver to be initialized at subsys_initcall than device_initcall

Signed-off-by Ray Jui <[email protected]>

---
 drivers/dma/pl330.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

drivers/dma/pl330.c +11 -1

index d5149aa..abb4cae 100644
@@ -2811,7 +2811,17 @@ static struct amba_driver pl330_driver = {
 	.remove = pl330_remove,
 };
 
-module_amba_driver(pl330_driver);
+static int __init pl330_init(void)
+{
+	return amba_driver_register(&pl330_driver);
+}
+subsys_initcall(pl330_init);
+
+static void __exit pl330_exit(void)
+{
+	amba_driver_unregister(&pl330_driver);
+}
+module_exit(pl330_exit);
 
 MODULE_AUTHOR("Jaswinder Singh <[email protected]>");
 MODULE_DESCRIPTION("API Driver for PL330 DMAC");

Device tree binding documentation for Broadcom Cygnus pinctrl driver

Signed-off-by Ray Jui <[email protected]>

Reviewed-by Scott Branden <[email protected]>

---
 .../bindings/pinctrl/brcm,cygnus-pinctrl.txt       |   92 ++++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,cygnus-pinctrl.txt

Documentation/devicetree/bindings/pinctrl/brcm,cygnus-pinctrl.txt new file +4 -0

new file mode 100644
index 0000000..86e4579
@@ -0,0 +1,92 @@
+Broadcom Cygnus Pin Controller
+
+The Cygnus pin controller supports setting the alternate functions of groups
+of pins. Pinmux configuration on individual pins is not supported by the

Quoted 24 lines · Ray Jui

Device tree binding documentation for Broadcom Cygnus pinctrl driver

Signed-off-by Ray Jui <[email protected]>

Reviewed-by Scott Branden <[email protected]>

---
 .../bindings/pinctrl/brcm,cygnus-pinctrl.txt       |   92 ++++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,cygnus-pinctrl.txt

Documentation/devicetree/bindings/pinctrl/brcm,cygnus-pinctrl.txt new file +9 -0

new file mode 100644
index 0000000..86e4579
@@ -0,0 +1,92 @@
+Broadcom Cygnus Pin Controller
+
+The Cygnus pin controller supports setting the alternate functions of groups
+of pins. Pinmux configuration on individual pins is not supported by the
+Cygnus A0 SoC.
+
+Required properties:
+
+- compatible:

Documentation/devicetree/bindings/pinctrl/brcm,cygnus-pinctrl.txt line 14

Quoted 5 lines

Documentation/devicetree/bindings/pinctrl/brcm,cygnus-pinctrl.txt new file +5 -0

+    Must be "brcm,cygnus-pinctrl"
+
+- reg:
+    Define the base and range of the I/O address space that contain the Cygnus
+pin control registers

The following is subnodes. Indicate clearly in the binding that these are *not* properties of the main node, but individual subnodes.

Documentation/devicetree/bindings/pinctrl/brcm,cygnus-pinctrl.txt line 22

Quoted 7 lines

Documentation/devicetree/bindings/pinctrl/brcm,cygnus-pinctrl.txt new file +7 -0

+- brcm,groups:
+    This can be strings of one or more group names. This defines the group(s)
+that one wants to configure
+
+- brcm,function:
+    This is the alternate function that one wants to configure to. Valid
+alternate functions are "alt1", "alt2", "alt3", "alt4"

NAK. We have standardized bindings for groups and functions, and there are pending patches from S?ren Brinkmann adding this to the pinctrl DT parsing core.

Just use "groups" and "function" and refer to Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Then "alt1", "alt2" etc are non-functional names of functions. Use the real function names, like "spi0" or so. This alt-business seems to be just a shortcut to make it simple, don't do that.

Then you use e.g. "spi0" as a group name. I prefer you call that "spi0_grp" or something to say it is a group of pins associated with spi0, as spi0 is actually the function.

If unsure of the definitions of group and function, refer to Documentation/pinctrl.txt

Yours,
Linus Walleij

Quoted 8 lines · Linus Walleij

Just use "groups" and "function" and refer to Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Then "alt1", "alt2" etc are non-functional names of functions. Use the real function names, like "spi0" or so. This alt-business seems to be just a shortcut to make it simple, don't do that.

Quoted 5 lines

Then you use e.g. "spi0" as a group name. I prefer you call that "spi0_grp" or something to say it is a group of pins associated with spi0, as spi0 is actually the function.

Hmmm, I did this by following brcm,bcm11351-pinctrl.txt:
- function: String. Specifies the pin mux selection. Values must be one
of: "alt1", "alt2", "alt3", "alt4"

But you are right, in the pinctrl binding document it describes the generic pin multiplexing nodes use "function" and "group".

For example, the group "lcd" covers 30 pins. When I configure "lcd" to alternate function 1, all 30 pins are muxed to LCD function. When configured to function 2, all pins are muxed to SRAM function. Now, here comes the issue, when configured to function 3, pins 1-15 and 20-30 become GPIO function, but pins 16-19 becomes SPI5 function. When it's configured to function 4, all 30 pins become GPIO.

In some other cases, when I configure a group to other functions, there could be spare pins which become unused (not brought out to the pad). Or, the spare pins may also become a separate function.

Based on the LCD example, I'd assume I would do the following for the default LCD function:

lcd_node {
        group = "lcd_grp";
        function = "lcd";
};

And in the case of function 3, I would call the function "spi5" and assume the rest of pins become either GPIO (or unused)?

spi5_node {
        group = "lcd_grp";
        function = "spi5";
};

Quoted 15 lines · Ray Jui

Quoted 8 lines · Linus Walleij

Just use "groups" and "function" and refer to Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Then "alt1", "alt2" etc are non-functional names of functions. Use the real function names, like "spi0" or so. This alt-business seems to be just a shortcut to make it simple, don't do that.

Quoted 5 lines

Then you use e.g. "spi0" as a group name. I prefer you call that "spi0_grp" or something to say it is a group of pins associated with spi0, as spi0 is actually the function.

Quoted 6 lines

Hmmm, I did this by following brcm,bcm11351-pinctrl.txt:
- function: String. Specifies the pin mux selection. Values must be one
of: "alt1", "alt2", "alt3", "alt4"

But you are right, in the pinctrl binding document it describes the generic pin multiplexing nodes use "function" and "group".

Note "function" and "groups". Note groups is pluralis. You can select multiple groups for a single function.

Quoted 6 lines

For example, the group "lcd" covers 30 pins. When I configure "lcd" to alternate function 1, all 30 pins are muxed to LCD function. When configured to function 2, all pins are muxed to SRAM function. Now, here comes the issue, when configured to function 3, pins 1-15 and 20-30 become GPIO function, but pins 16-19 becomes SPI5 function. When it's configured to function 4, all 30 pins become GPIO.

I would split the use case in two groups for LCD and SRAM, and three groups for GPIO:
"lcd_grp", "sram_grp", "gpio-1-15_grp", "gpio-16-19_grp", "gpio-20-30_grp", "spi5_grp"

Valid combinations become

function = "lcd"
groups = "lcd_grp";

function = "sram"
groups = "sram_grp"

For all GPIO only this:

function = "gpio"
groups = "gpio-1-16_grp", "gpio-16-19_grp", "gpio-20-30_grp"

For a combined GPIO+SPI:

function = "gpio"
groups = "gpio-1-16_grp", "gpio-20-30_grp"

function = "spi5"
groups = "spi5_grp"

The pinctrl runtile will protest if you try to combine spi5_grp with gpio-16-19_grp.

Quoted 3 lines

In some other cases, when I configure a group to other functions, there could be spare pins which become unused (not brought out to the pad). Or, the spare pins may also become a separate function.

That's cool.

Quoted 8 lines

Based on the LCD example, I'd assume I would do the following for the default LCD function:

lcd_node {
        group = "lcd_grp";
        function = "lcd";
};

Quoted 7 lines

And in the case of function 3, I would call the function "spi5" and assume the rest of pins become either GPIO (or unused)?

spi5_node {
        group = "lcd_grp";
        function = "spi5";
};

Looks cool per above.

You need some clever code in the driver to handle double-configuration of registers and so on, but I think it can be done.

Using pin control as a GPIO backend can be a bit tricky and will need some testing and elaboration, but the subsystem will block collisions.

Yours,
Linus Walleij

Quoted 15 lines · Ray Jui

Quoted 8 lines · Linus Walleij

Just use "groups" and "function" and refer to Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Then "alt1", "alt2" etc are non-functional names of functions. Use the real function names, like "spi0" or so. This alt-business seems to be just a shortcut to make it simple, don't do that.

Quoted 5 lines

Then you use e.g. "spi0" as a group name. I prefer you call that "spi0_grp" or something to say it is a group of pins associated with spi0, as spi0 is actually the function.

Quoted 6 lines

Hmmm, I did this by following brcm,bcm11351-pinctrl.txt:
- function: String. Specifies the pin mux selection. Values must be one
of: "alt1", "alt2", "alt3", "alt4"

But you are right, in the pinctrl binding document it describes the generic pin multiplexing nodes use "function" and "group".

Note "function" and "groups". Note groups is pluralis. You can select multiple groups for a single function.

Quoted 6 lines

For example, the group "lcd" covers 30 pins. When I configure "lcd" to alternate function 1, all 30 pins are muxed to LCD function. When configured to function 2, all pins are muxed to SRAM function. Now, here comes the issue, when configured to function 3, pins 1-15 and 20-30 become GPIO function, but pins 16-19 becomes SPI5 function. When it's configured to function 4, all 30 pins become GPIO.

I would split the use case in two groups for LCD and SRAM, and three groups for GPIO:
"lcd_grp", "sram_grp", "gpio-1-15_grp", "gpio-16-19_grp", "gpio-20-30_grp", "spi5_grp"

Valid combinations become

function = "lcd"
groups = "lcd_grp";

function = "sram"
groups = "sram_grp"

For all GPIO only this:

function = "gpio"
groups = "gpio-1-16_grp", "gpio-16-19_grp", "gpio-20-30_grp"

For a combined GPIO+SPI:

function = "gpio"
groups = "gpio-1-16_grp", "gpio-20-30_grp"

function = "spi5"
groups = "spi5_grp"

The pinctrl runtile will protest if you try to combine spi5_grp with gpio-16-19_grp.

Quoted 3 lines

In some other cases, when I configure a group to other functions, there could be spare pins which become unused (not brought out to the pad). Or, the spare pins may also become a separate function.

That's cool.

Quoted 8 lines

Based on the LCD example, I'd assume I would do the following for the default LCD function:

lcd_node {
        group = "lcd_grp";
        function = "lcd";
};

Quoted 7 lines

And in the case of function 3, I would call the function "spi5" and assume the rest of pins become either GPIO (or unused)?

spi5_node {
        group = "lcd_grp";
        function = "spi5";
};

Looks cool per above.

You need some clever code in the driver to handle double-configuration of registers and so on, but I think it can be done.

Using pin control as a GPIO backend can be a bit tricky and will need some testing and elaboration, but the subsystem will block collisions.

Yours,
Linus Walleij

Quoted 86 lines · Linus Walleij

Quoted 15 lines · Ray Jui

Quoted 8 lines · Linus Walleij

Just use "groups" and "function" and refer to Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt

Then "alt1", "alt2" etc are non-functional names of functions. Use the real function names, like "spi0" or so. This alt-business seems to be just a shortcut to make it simple, don't do that.

Quoted 5 lines

Then you use e.g. "spi0" as a group name. I prefer you call that "spi0_grp" or something to say it is a group of pins associated with spi0, as spi0 is actually the function.

Quoted 6 lines

Hmmm, I did this by following brcm,bcm11351-pinctrl.txt:
- function: String. Specifies the pin mux selection. Values must be one
of: "alt1", "alt2", "alt3", "alt4"

But you are right, in the pinctrl binding document it describes the generic pin multiplexing nodes use "function" and "group".

Note "function" and "groups". Note groups is pluralis. You can select multiple groups for a single function.

Quoted 6 lines

For example, the group "lcd" covers 30 pins. When I configure "lcd" to alternate function 1, all 30 pins are muxed to LCD function. When configured to function 2, all pins are muxed to SRAM function. Now, here comes the issue, when configured to function 3, pins 1-15 and 20-30 become GPIO function, but pins 16-19 becomes SPI5 function. When it's configured to function 4, all 30 pins become GPIO.

I would split the use case in two groups for LCD and SRAM, and three groups for GPIO:
"lcd_grp", "sram_grp", "gpio-1-15_grp", "gpio-16-19_grp", "gpio-20-30_grp", "spi5_grp"

Valid combinations become

function = "lcd"
groups = "lcd_grp";

function = "sram"
groups = "sram_grp"

For all GPIO only this:

function = "gpio"
groups = "gpio-1-16_grp", "gpio-16-19_grp", "gpio-20-30_grp"

For a combined GPIO+SPI:

function = "gpio"
groups = "gpio-1-16_grp", "gpio-20-30_grp"

function = "spi5"
groups = "spi5_grp"

The pinctrl runtile will protest if you try to combine spi5_grp with gpio-16-19_grp.

Quoted 3 lines

In some other cases, when I configure a group to other functions, there could be spare pins which become unused (not brought out to the pad). Or, the spare pins may also become a separate function.

That's cool.

Quoted 8 lines

Based on the LCD example, I'd assume I would do the following for the default LCD function:

lcd_node {
        group = "lcd_grp";
        function = "lcd";
};

Quoted 7 lines

And in the case of function 3, I would call the function "spi5" and assume the rest of pins become either GPIO (or unused)?

spi5_node {
        group = "lcd_grp";
        function = "spi5";
};

Looks cool per above.

Quoted 9 lines

You need some clever code in the driver to handle double-configuration of registers and so on, but I think it can be done.

Using pin control as a GPIO backend can be a bit tricky and will need some testing and elaboration, but the subsystem will block collisions.

Yours,
Linus Walleij

Hi Linus,

I have another question here. In the B0 revision of our Cygnus chip, the ASIC team added a feature to allow individual pins to be muxed to GPIO. The pinmux controller can still only do group-based muxing in general, but at the same time, you can override most (but not all) individual pins to GPIO.

I believe this HW design actually forces us to mix use "groups" and "pins" in DT.

For example, assuming we mux pins 1 - 10 as MMC (one cmd line, one clk line, and 8 data lines). One might make the decision that he only needs 4 data lines instead of 8 data lines, and he wants to free up the 4 data lines and uses as GPIO. Based on this example, is the following DT configuration valid?

sd_node {
    function = "sd";
    groups = "sd_grps";
};

gpio_node {
    function = "gpio";
    pins = "gpio_7", "gpio_8", "gpio_9", "gpio_10"; /* assuming 1:1

mapping between gpio and pin number to make this example simple */ };

Thanks,

Ray

I dig into the pinctrl framework code a bit more and found that I can use pinctrl_request_gpio from the GPIO driver and implement gpio_request_enable in the pinctrl driver.

The only problem I see now is that these APIs seem to expect the use of global GPIO numbers?

I hope I'm not missing something here?

Quoted 3 lines · Ray Jui

I dig into the pinctrl framework code a bit more and found that I can use pinctrl_request_gpio from the GPIO driver and implement gpio_request_enable in the pinctrl driver.

Yep :) ain't it nice.

Quoted 2 lines

The only problem I see now is that these APIs seem to expect the use of global GPIO numbers?

No they don't, only if you use the deprecated pinctrl_add_gpio_range().

Instead, when you register your struct gpio_chip, use gpiochip_add_pin_range() and this will use relative offsets without relying on global GPIO numbers.

This latter call replaces pinctrl_add_gpio_range().

Quoted 1 line

I hope I'm not missing something here?

You're missing gpiochip_add_pin_range() ;)

Yours,
Linus Walleij

I have another question here. In the B0 revision of our Cygnus chip, the ASIC team added a feature to allow individual pins to be muxed to GPIO. The pinmux controller can still only do group-based muxing in general, but at the same time, you can override most (but not all) individual pins to GPIO.

I believe this HW design actually forces us to mix use "groups" and "pins" in DT.

For example, assuming we mux pins 1 - 10 as MMC (one cmd line, one clk line, and 8 data lines). One might make the decision that he only needs 4 data lines instead of 8 data lines, and he wants to free up the 4 data lines and uses as GPIO.

Based on this example, is the following DT
configuration valid?

sd_node {
    function = "sd";
    groups = "sd_grps";
};

gpio_node {
    function = "gpio";
    pins = "gpio_7", "gpio_8", "gpio_9", "gpio_10"; /* assuming 1:1

mapping between gpio and pin number to make this example simple */ };

This adds the initial driver support for the Broadcom Cygnus pinctrl controller. The Cygnus pinctrl controller supports group based alternate function configuration

Signed-off-by Ray Jui <[email protected]>

Reviewed-by Scott Branden <[email protected]>

Signed-off-by Fengguang Wu <[email protected]>

---
 drivers/pinctrl/Kconfig              |    7 +
 drivers/pinctrl/Makefile             |    1 +
 drivers/pinctrl/pinctrl-bcm-cygnus.c |  753 ++++++++++++++++++++++++++++++++++

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +5 -0

+/*
+ * Cygnus pinctrl core
+ *
+ * @pctl: pointer to pinctrl_dev
+ * @dev: pointer to the device

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +17 -0

+/*
+ * List of groups of pins
+ */
+static const unsigned gpio0_pins[] = { 12 };
+static const unsigned gpio1_pins[] = { 13 };
+static const unsigned gpio2_pins[] = { 14 };
+static const unsigned gpio3_pins[] = { 15 };
+static const unsigned gpio4_pins[] = { 16 };
+static const unsigned gpio5_pins[] = { 17 };
+static const unsigned gpio6_pins[] = { 18 };
+static const unsigned gpio7_pins[] = { 19 };
+static const unsigned gpio8_pins[] = { 20 };
+static const unsigned gpio9_pins[] = { 21 };
+static const unsigned gpio10_pins[] = { 22 };
+static const unsigned gpio11_pins[] = { 23 };
+static const unsigned gpio12_pins[] = { 24 };
+static const unsigned gpio13_pins[] = { 25 };

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +30 -0

+static const unsigned pwm0_pins[] = { 38 };
+static const unsigned pwm1_pins[] = { 39 };
+static const unsigned pwm2_pins[] = { 40 };
+static const unsigned pwm3_pins[] = { 41 };
+static const unsigned sdio0_pins[] = { 94, 95, 96, 97, 98, 99 };
+static const unsigned smart_card0_pins[] = { 42, 43, 44, 46, 47 };
+static const unsigned smart_card1_pins[] = { 48, 49, 50, 52, 53 };
+static const unsigned spi0_pins[] = { 54, 55, 56, 57 };
+static const unsigned spi1_pins[] = { 58, 59, 60, 61 };
+static const unsigned spi2_pins[] = { 62, 63, 64, 65 };
+static const unsigned spi3_pins[] = { 66, 67, 68, 69 };
+static const unsigned d1w_pins[] = { 10, 11 };
+static const unsigned lcd_pins[] = { 126, 127, 128, 129, 130, 131, 132,        133,
+       134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147,
+       148, 149, 150, 151, 152, 153, 154, 155 };
+static const unsigned uart0_pins[] = { 70, 71, 72, 73 };
+static const unsigned uart1_dte_pins[] = { 75, 76, 77, 78 };
+static const unsigned uart1_pins[] = { 74, 79, 80, 81 };
+static const unsigned uart3_pins[] = { 82, 83 };
+static const unsigned qspi_pins[] = { 104, 105, 106, 107 };
+static const unsigned nand_pins[] = { 110, 111, 112, 113, 114, 115, 116, 117,
+       118, 119, 120, 121, 122, 123, 124, 125 };
+static const unsigned sdio0_cd_pins[] = { 103 };
+static const unsigned sdio0_mmc_pins[] = { 100, 101, 102 };
+static const unsigned can0_spi4_pins[] = { 86, 87 };
+static const unsigned can1_spi4_pins[] = { 88, 89 };
+static const unsigned sdio1_cd_pins[] = { 93 };
+static const unsigned sdio1_led_pins[] = { 84, 85 };
+static const unsigned sdio1_mmc_pins[] = { 90, 91, 92 };
+static const unsigned camera_led_pins[] = { 156, 157, 158, 159, 160 };

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +54 -0

+/*
+ * List of groups names. Need to match the order in cygnus_pin_groups
+ */
+static const char * const cygnus_pin_group_names[] = {
+       "gpio0",
+       "gpio1",
+       "gpio2",
+       "gpio3",
+       "gpio4",
+       "gpio5",
+       "gpio6",
+       "gpio7",
+       "gpio8",
+       "gpio9",
+       "gpio10",
+       "gpio11",
+       "gpio12",
+       "gpio13",
+       "gpio14",
+       "gpio15",
+       "gpio16",
+       "gpio17",
+       "gpio18",
+       "gpio19",
+       "gpio20",
+       "gpio21",
+       "gpio22",
+       "gpio23",
+       "pwm0",
+       "pwm1",
+       "pwm2",
+       "pwm3",
+       "sdio0",
+       "smart_card0",
+       "smart_card1",
+       "spi0",
+       "spi1",
+       "spi2",
+       "spi3",
+       "d1w",
+       "lcd",
+       "uart0",
+       "uart1_dte",
+       "uart1",
+       "uart3",
+       "qspi",
+       "nand",
+       "sdio0_cd",
+       "sdio0_mmc",
+       "can0_spi4",
+       "can1_spi4",
+       "sdio1_cd",
+       "sdio1_led",
+       "sdio1_mmc",

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +8 -0

+#define CYGNUS_PIN_FUNCTION(fcn_name, mux_val)                 \
+{                                                              \
+       .name = #fcn_name,                                      \
+       .group_names = cygnus_pin_group_names,                  \
+       .num_groups = ARRAY_SIZE(cygnus_pin_group_names),       \
+       .mux = mux_val,                                         \
+}
+

+ num_groups = of_property_count_strings(np, "brcm,groups");

+ ret = of_property_read_string(np, "brcm,function", &function_name);

Quoted 11 lines · Ray Jui

This adds the initial driver support for the Broadcom Cygnus pinctrl controller. The Cygnus pinctrl controller supports group based alternate function configuration

Signed-off-by Ray Jui <[email protected]>

Reviewed-by Scott Branden <[email protected]>

Signed-off-by Fengguang Wu <[email protected]>

---
 drivers/pinctrl/Kconfig              |    7 +
 drivers/pinctrl/Makefile             |    1 +
 drivers/pinctrl/pinctrl-bcm-cygnus.c |  753 ++++++++++++++++++++++++++++++++++

With the proliferation of Broadcom drivers, please first send a patch moving pinctrl-bcm281xx.c and pinctrl-bcm2835.c to drivers/pinctrl/broadcom or something, so we can collect them there.

I don't know if the hardware has any similarity though, so invite the authors of the previous drivers to review this code.

drivers/pinctrl/Kconfig config PINCTRL_BCM281XX · line 93

Quoted 6 lines

drivers/pinctrl/Kconfig +6 -0

+config PINCTRL_BCM_CYGNUS
+       bool "Broadcom Cygnus pinctrl driver"
+       depends on (ARCH_BCM_CYGNUS || COMPILE_TEST)
+       select PINMUX
+       select PINCONF
+       select GENERIC_PINCONF

Nice that you use GENERIC_PINCONF! :)

Quoted 10 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +10 -0

+/*
+ * Cygnus pinctrl core
+ *
+ * @pctl: pointer to pinctrl_dev
+ * @dev: pointer to the device
+ * @base: I/O register base for Cygnus pinctrl configuration
+ *
+ */
+struct cygnus_pinctrl {
+       struct pinctrl_dev *pctl;

drivers/pinctrl/pinctrl-bcm-cygnus.c line 76

Quoted 5 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +5 -0

+       struct device *dev;
+       void __iomem *base;
+
+       const struct pinctrl_pin_desc *pins;
+       unsigned num_pins;

Why is this not simply just a part of struct pinctrl_desc? Why does it have to be multiplied here?

Quoted 23 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +23 -0

+/*
+ * List of groups of pins
+ */
+static const unsigned gpio0_pins[] = { 12 };
+static const unsigned gpio1_pins[] = { 13 };
+static const unsigned gpio2_pins[] = { 14 };
+static const unsigned gpio3_pins[] = { 15 };
+static const unsigned gpio4_pins[] = { 16 };
+static const unsigned gpio5_pins[] = { 17 };
+static const unsigned gpio6_pins[] = { 18 };
+static const unsigned gpio7_pins[] = { 19 };
+static const unsigned gpio8_pins[] = { 20 };
+static const unsigned gpio9_pins[] = { 21 };
+static const unsigned gpio10_pins[] = { 22 };
+static const unsigned gpio11_pins[] = { 23 };
+static const unsigned gpio12_pins[] = { 24 };
+static const unsigned gpio13_pins[] = { 25 };
+static const unsigned gpio14_pins[] = { 26 };
+static const unsigned gpio15_pins[] = { 27 };
+static const unsigned gpio16_pins[] = { 28 };
+static const unsigned gpio17_pins[] = { 29 };
+static const unsigned gpio18_pins[] = { 30 };
+static const unsigned gpio19_pins[] = { 31 };

drivers/pinctrl/pinctrl-bcm-cygnus.c line 306

Quoted 4 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +4 -0

+static const unsigned gpio20_pins[] = { 32 };
+static const unsigned gpio21_pins[] = { 33 };
+static const unsigned gpio22_pins[] = { 34 };
+static const unsigned gpio23_pins[] = { 35 };

Have you considered implementing .gpio_request_enable() and .gpio_disable_free() to get around having to have one group for each GPIO line?

Quoted 34 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +34 -0

+static const unsigned pwm0_pins[] = { 38 };
+static const unsigned pwm1_pins[] = { 39 };
+static const unsigned pwm2_pins[] = { 40 };
+static const unsigned pwm3_pins[] = { 41 };
+static const unsigned sdio0_pins[] = { 94, 95, 96, 97, 98, 99 };
+static const unsigned smart_card0_pins[] = { 42, 43, 44, 46, 47 };
+static const unsigned smart_card1_pins[] = { 48, 49, 50, 52, 53 };
+static const unsigned spi0_pins[] = { 54, 55, 56, 57 };
+static const unsigned spi1_pins[] = { 58, 59, 60, 61 };
+static const unsigned spi2_pins[] = { 62, 63, 64, 65 };
+static const unsigned spi3_pins[] = { 66, 67, 68, 69 };
+static const unsigned d1w_pins[] = { 10, 11 };
+static const unsigned lcd_pins[] = { 126, 127, 128, 129, 130, 131, 132,        133,
+       134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147,
+       148, 149, 150, 151, 152, 153, 154, 155 };
+static const unsigned uart0_pins[] = { 70, 71, 72, 73 };
+static const unsigned uart1_dte_pins[] = { 75, 76, 77, 78 };
+static const unsigned uart1_pins[] = { 74, 79, 80, 81 };
+static const unsigned uart3_pins[] = { 82, 83 };
+static const unsigned qspi_pins[] = { 104, 105, 106, 107 };
+static const unsigned nand_pins[] = { 110, 111, 112, 113, 114, 115, 116, 117,
+       118, 119, 120, 121, 122, 123, 124, 125 };
+static const unsigned sdio0_cd_pins[] = { 103 };
+static const unsigned sdio0_mmc_pins[] = { 100, 101, 102 };
+static const unsigned can0_spi4_pins[] = { 86, 87 };
+static const unsigned can1_spi4_pins[] = { 88, 89 };
+static const unsigned sdio1_cd_pins[] = { 93 };
+static const unsigned sdio1_led_pins[] = { 84, 85 };
+static const unsigned sdio1_mmc_pins[] = { 90, 91, 92 };
+static const unsigned camera_led_pins[] = { 156, 157, 158, 159, 160 };
+static const unsigned camera_rgmii_pins[] = { 169, 170, 171, 169, 170, 171,
+       172, 173 };
+static const unsigned camera_sram_rgmii_pins[] = { 161, 162, 163, 164, 165,
+       166, 167, 168 };

drivers/pinctrl/pinctrl-bcm-cygnus.c line 346

Quoted 6 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +6 -0

+static const unsigned qspi_gpio_pins[] = { 108, 109 };
+static const unsigned smart_card0_fcb_pins[] = { 45 };
+static const unsigned smart_card1_fcb_pins[] = { 51 };
+static const unsigned gpio0_3p3_pins[] = { 176 };
+static const unsigned gpio1_3p3_pins[] = { 177 };
+static const unsigned gpio2_3p3_pins[] = { 178 };

Looks good...

Quoted 61 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +61 -0

+/*
+ * List of groups names. Need to match the order in cygnus_pin_groups
+ */
+static const char * const cygnus_pin_group_names[] = {
+       "gpio0",
+       "gpio1",
+       "gpio2",
+       "gpio3",
+       "gpio4",
+       "gpio5",
+       "gpio6",
+       "gpio7",
+       "gpio8",
+       "gpio9",
+       "gpio10",
+       "gpio11",
+       "gpio12",
+       "gpio13",
+       "gpio14",
+       "gpio15",
+       "gpio16",
+       "gpio17",
+       "gpio18",
+       "gpio19",
+       "gpio20",
+       "gpio21",
+       "gpio22",
+       "gpio23",
+       "pwm0",
+       "pwm1",
+       "pwm2",
+       "pwm3",
+       "sdio0",
+       "smart_card0",
+       "smart_card1",
+       "spi0",
+       "spi1",
+       "spi2",
+       "spi3",
+       "d1w",
+       "lcd",
+       "uart0",
+       "uart1_dte",
+       "uart1",
+       "uart3",
+       "qspi",
+       "nand",
+       "sdio0_cd",
+       "sdio0_mmc",
+       "can0_spi4",
+       "can1_spi4",
+       "sdio1_cd",
+       "sdio1_led",
+       "sdio1_mmc",
+       "camera_led",
+       "camera_rgmii",
+       "camera_sram_rgmii",
+       "qspi_gpio",
+       "smart_card0_fcb",
+       "smart_card1_fcb",
+       "gpio0_3p3",

drivers/pinctrl/pinctrl-bcm-cygnus.c line 411

Quoted 3 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +3 -0

+       "gpio1_3p3",
+       "gpio2_3p3",
+};

This looks very much like function names as noted in the binding. I would say, suffix every group with _grp or something so it's not as confusing. Remember, spi0 is a function of the SoC, pins {1,2} is just a group of pins that it may appear on.

Quoted 8 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +8 -0

+#define CYGNUS_PIN_FUNCTION(fcn_name, mux_val)                 \
+{                                                              \
+       .name = #fcn_name,                                      \
+       .group_names = cygnus_pin_group_names,                  \
+       .num_groups = ARRAY_SIZE(cygnus_pin_group_names),       \
+       .mux = mux_val,                                         \
+}
+

drivers/pinctrl/pinctrl-bcm-cygnus.c line 495

Quoted 10 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +10 -0

+/*
+ * Cygnus has 4 alternate functions. All groups can be configured to any of
+ * the 4 alternate functions
+ */
+static const struct cygnus_pin_function cygnus_pin_functions[] = {
+       CYGNUS_PIN_FUNCTION(alt1, 0),
+       CYGNUS_PIN_FUNCTION(alt2, 1),
+       CYGNUS_PIN_FUNCTION(alt3, 2),
+       CYGNUS_PIN_FUNCTION(alt4, 3),
+};

These are not functions. These are per-pin mux ways.

Re-read the documentation of what a function is: it is not something abstract like "alternative something" but something very direct like uart0 or spi0.

drivers/pinctrl/pinctrl-bcm-cygnus.c line 545

Quoted 4 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +4 -0

+static int cygnus_dt_node_to_map(struct pinctrl_dev *pctrl_dev,
+               struct device_node *np, struct pinctrl_map **map,
+               unsigned *num_maps)
+{

After S?ren Brinkmanns patches youy should be able to use core functions for this and avoid this code altogether.

Quoted 1 line

+ num_groups = of_property_count_strings(np, "brcm,groups");

As mentioned, just "groups".

Quoted 1 line

+ ret = of_property_read_string(np, "brcm,function", &function_name);

As mentioned, just "function".

drivers/pinctrl/pinctrl-bcm-cygnus.c line 576

Quoted 2 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +2 -0

+       ret = pinctrl_utils_reserve_map(pctrl_dev, map, &reserved_maps,
+                       num_maps, num_groups);

Good use of utilities!

Apart from this things look nice.

The main comment to address is the definition of functions.

Yours,
Linus Walleij

Quoted 5 lines · Linus Walleij

On Fri, Nov 28, 2014 at 12:46 AM, Ray Jui <[email protected]> wrote:

I don't know if the hardware has any similarity though, so invite the authors of the previous drivers to review this code.

They are completely different. The only similarity between Cygnus and bcm281xx pinctrl is that they use the same concept of alternation functions (1, 2, 3, 4) for mux configuration.

Quoted 23 lines

Quoted 23 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +23 -0

+/*
+ * List of groups of pins
+ */
+static const unsigned gpio0_pins[] = { 12 };
+static const unsigned gpio1_pins[] = { 13 };
+static const unsigned gpio2_pins[] = { 14 };
+static const unsigned gpio3_pins[] = { 15 };
+static const unsigned gpio4_pins[] = { 16 };
+static const unsigned gpio5_pins[] = { 17 };
+static const unsigned gpio6_pins[] = { 18 };
+static const unsigned gpio7_pins[] = { 19 };
+static const unsigned gpio8_pins[] = { 20 };
+static const unsigned gpio9_pins[] = { 21 };
+static const unsigned gpio10_pins[] = { 22 };
+static const unsigned gpio11_pins[] = { 23 };
+static const unsigned gpio12_pins[] = { 24 };
+static const unsigned gpio13_pins[] = { 25 };
+static const unsigned gpio14_pins[] = { 26 };
+static const unsigned gpio15_pins[] = { 27 };
+static const unsigned gpio16_pins[] = { 28 };
+static const unsigned gpio17_pins[] = { 29 };
+static const unsigned gpio18_pins[] = { 30 };
+static const unsigned gpio19_pins[] = { 31 };

Quoted 9 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c line 306

Quoted 4 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +4 -0

+static const unsigned gpio20_pins[] = { 32 };
+static const unsigned gpio21_pins[] = { 33 };
+static const unsigned gpio22_pins[] = { 34 };
+static const unsigned gpio23_pins[] = { 35 };

Have you considered implementing .gpio_request_enable() and .gpio_disable_free() to get around having to have one group for each GPIO line?

Okay the Cygnus pin controller is really a mess. GPIO 0 ~ GPIO23 are really 23 distinct groups, each with one pin. Then the rest of GPIOs go under other groups. In general, when we set a group to alternate function 4, all pins become GPIO.

Quoted 9 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c line 346

Quoted 6 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +6 -0

+static const unsigned qspi_gpio_pins[] = { 108, 109 };
+static const unsigned smart_card0_fcb_pins[] = { 45 };
+static const unsigned smart_card1_fcb_pins[] = { 51 };
+static const unsigned gpio0_3p3_pins[] = { 176 };
+static const unsigned gpio1_3p3_pins[] = { 177 };
+static const unsigned gpio2_3p3_pins[] = { 178 };

Looks good...

Note these pins are definitions in the driver that help to describe the pad layout. We can't really configure any individual pins in Cygnus.

Quoted 7 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c line 545

Quoted 4 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +4 -0

+static int cygnus_dt_node_to_map(struct pinctrl_dev *pctrl_dev,
+               struct device_node *np, struct pinctrl_map **map,
+               unsigned *num_maps)
+{

After S?ren Brinkmanns patches youy should be able to use core functions for this and avoid this code altogether.

Will that help to take care our case, based on the way we will use "function" and "group"?

Quoted 4 lines

Quoted 1 line

+ num_groups = of_property_count_strings(np, "brcm,groups");

As mentioned, just "groups".

I guess I will use "group"?

Quoted 9 lines · Ray Jui

Quoted 5 lines · Linus Walleij

On Fri, Nov 28, 2014 at 12:46 AM, Ray Jui <[email protected]> wrote:

I don't know if the hardware has any similarity though, so invite the authors of the previous drivers to review this code.

They are completely different. The only similarity between Cygnus and bcm281xx pinctrl is that they use the same concept of alternation functions (1, 2, 3, 4) for mux configuration.

Then you can probably look at that driver for inspiration on how to handle the situation you described earlier with collissions.

Quoted 32 lines

Quoted 23 lines

Quoted 23 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +23 -0

+/*
+ * List of groups of pins
+ */
+static const unsigned gpio0_pins[] = { 12 };
+static const unsigned gpio1_pins[] = { 13 };
+static const unsigned gpio2_pins[] = { 14 };
+static const unsigned gpio3_pins[] = { 15 };
+static const unsigned gpio4_pins[] = { 16 };
+static const unsigned gpio5_pins[] = { 17 };
+static const unsigned gpio6_pins[] = { 18 };
+static const unsigned gpio7_pins[] = { 19 };
+static const unsigned gpio8_pins[] = { 20 };
+static const unsigned gpio9_pins[] = { 21 };
+static const unsigned gpio10_pins[] = { 22 };
+static const unsigned gpio11_pins[] = { 23 };
+static const unsigned gpio12_pins[] = { 24 };
+static const unsigned gpio13_pins[] = { 25 };
+static const unsigned gpio14_pins[] = { 26 };
+static const unsigned gpio15_pins[] = { 27 };
+static const unsigned gpio16_pins[] = { 28 };
+static const unsigned gpio17_pins[] = { 29 };
+static const unsigned gpio18_pins[] = { 30 };
+static const unsigned gpio19_pins[] = { 31 };

Quoted 9 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c line 306

Quoted 4 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +4 -0

+static const unsigned gpio20_pins[] = { 32 };
+static const unsigned gpio21_pins[] = { 33 };
+static const unsigned gpio22_pins[] = { 34 };
+static const unsigned gpio23_pins[] = { 35 };

Have you considered implementing .gpio_request_enable() and .gpio_disable_free() to get around having to have one group for each GPIO line?

Quoted 4 lines

Okay the Cygnus pin controller is really a mess. GPIO 0 ~ GPIO23 are really 23 distinct groups, each with one pin. Then the rest of GPIOs go under other groups. In general, when we set a group to alternate function 4, all pins become GPIO.

It will require some complicated code no matter how you handle it I'm afraid. Rely on the pin control subsystem to handle collisions though.

Quoted 9 lines

Quoted 9 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c line 346

Quoted 6 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +6 -0

+static const unsigned qspi_gpio_pins[] = { 108, 109 };
+static const unsigned smart_card0_fcb_pins[] = { 45 };
+static const unsigned smart_card1_fcb_pins[] = { 51 };
+static const unsigned gpio0_3p3_pins[] = { 176 };
+static const unsigned gpio1_3p3_pins[] = { 177 };
+static const unsigned gpio2_3p3_pins[] = { 178 };

Looks good...

Quoted 2 lines

Note these pins are definitions in the driver that help to describe the pad layout. We can't really configure any individual pins in Cygnus.

Yeah it's a groupwise controller then, that's similar to e.g. the coh901 driver.

We should be able to accomodate this...

Quoted 8 lines

Quoted 7 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c line 545

Quoted 4 lines

drivers/pinctrl/pinctrl-bcm-cygnus.c new file +4 -0

+static int cygnus_dt_node_to_map(struct pinctrl_dev *pctrl_dev,
+               struct device_node *np, struct pinctrl_map **map,
+               unsigned *num_maps)
+{

After S?ren Brinkmanns patches youy should be able to use core functions for this and avoid this code altogether.

Quoted 2 lines

Will that help to take care our case, based on the way we will use "function" and "group"?

groupS but yes it will work with your controller, though I think .set_mux and the controller state will need some elaborate code to handle what the framework requests.

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

+ num_groups = of_property_count_strings(np, "brcm,groups");

As mentioned, just "groups".

I guess I will use "group"?

No groups, as with the standard attribute "gpios", this may be a single group too, it's just a standard binding.

Yours,
Linus Walleij

This adds basic and generic support for various iProc PLLs and clocks including the ARMPLL, GENPLL, LCPLL, MIPIPLL, and ASIU clocks.

SoCs under the iProc architecture can define their specific register offsets and clock parameters for their PLL and clock controllers. These parameters can be passed as arugments into the generic iProc PLL and clock setup functions

Derived from code originally provided by Jonathan Richardson <[email protected]>

Signed-off-by Ray Jui <[email protected]>

Reviewed-by Scott Branden <[email protected]>

---
 drivers/clk/Makefile               |    2 +-
 drivers/clk/bcm/Kconfig            |    9 +
 drivers/clk/bcm/Makefile           |    1 +
 drivers/clk/bcm/clk-iproc-armpll.c |  286 +++++++++++++++++++++
 drivers/clk/bcm/clk-iproc-asiu.c   |  275 ++++++++++++++++++++
 drivers/clk/bcm/clk-iproc-clk.c    |  238 ++++++++++++++++++
 drivers/clk/bcm/clk-iproc-pll.c    |  483 ++++++++++++++++++++++++++++++++++++
 drivers/clk/bcm/clk-iproc.h        |  155 ++++++++++++
 8 files changed, 1448 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/bcm/clk-iproc-armpll.c
 create mode 100644 drivers/clk/bcm/clk-iproc-asiu.c
 create mode 100644 drivers/clk/bcm/clk-iproc-clk.c
 create mode 100644 drivers/clk/bcm/clk-iproc-pll.c
 create mode 100644 drivers/clk/bcm/clk-iproc.h

drivers/clk/Makefile +0 -0

index d5fba5b..eff0213 100644

drivers/clk/bcm/Kconfig +9 -0

index 75506e5..66b5b7f 100644
@@ -7,3 +7,12 @@ config CLK_BCM_KONA
          Enable common clock framework support for Broadcom SoCs
          using "Kona" style clock control units, including those
          in the BCM281xx and BCM21664 families.
+
+config COMMON_CLK_IPROC
+       bool "Broadcom iProc clock support"
+       depends on ARCH_BCM_IPROC
+       depends on COMMON_CLK
+       default y
+       help
+         Enable common clock framework support for Broadcom SoCs
+         based on the "iProc" architecture

drivers/clk/bcm/Makefile +1 -0

index 6297d05..6926636 100644
@@ -2,3 +2,4 @@ obj-$(CONFIG_CLK_BCM_KONA)      += clk-kona.o
 obj-$(CONFIG_CLK_BCM_KONA)     += clk-kona-setup.o
 obj-$(CONFIG_CLK_BCM_KONA)     += clk-bcm281xx.o
 obj-$(CONFIG_CLK_BCM_KONA)     += clk-bcm21664.o
+obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-clk.o clk-iproc-asiu.o

drivers/clk/bcm/clk-iproc-armpll.c new file +286 -0

new file mode 100644
index 0000000..ec9b130
@@ -0,0 +1,286 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+
+#define IPROC_CLK_MAX_FREQ_POLICY                    0x3
+#define IPROC_CLK_POLICY_FREQ_OFFSET                 0x008
+#define IPROC_CLK_POLICY_FREQ_POLICY_FREQ_SHIFT      8
+#define IPROC_CLK_POLICY_FREQ_POLICY_FREQ_MASK       0x7
+
+#define IPROC_CLK_PLLARMA_OFFSET                     0xc00
+#define IPROC_CLK_PLLARMA_LOCK_SHIFT                 28
+#define IPROC_CLK_PLLARMA_PDIV_SHIFT                 24
+#define IPROC_CLK_PLLARMA_PDIV_MASK                  0xf
+#define IPROC_CLK_PLLARMA_NDIV_INT_SHIFT             8
+#define IPROC_CLK_PLLARMA_NDIV_INT_MASK              0x3ff
+
+#define IPROC_CLK_PLLARMB_OFFSET                     0xc04
+#define IPROC_CLK_PLLARMB_NDIV_FRAC_MASK             0xfffff
+
+#define IPROC_CLK_PLLARMC_OFFSET                     0xc08
+#define IPROC_CLK_PLLARMC_BYPCLK_EN_SHIFT            8
+#define IPROC_CLK_PLLARMC_MDIV_MASK                  0xff
+
+#define IPROC_CLK_PLLARMCTL5_OFFSET                  0xc20
+#define IPROC_CLK_PLLARMCTL5_H_MDIV_MASK             0xff
+
+#define IPROC_CLK_PLLARM_OFFSET_OFFSET               0xc24
+#define IPROC_CLK_PLLARM_SW_CTL_SHIFT                29
+#define IPROC_CLK_PLLARM_NDIV_INT_OFFSET_SHIFT       20
+#define IPROC_CLK_PLLARM_NDIV_INT_OFFSET_MASK        0xff
+#define IPROC_CLK_PLLARM_NDIV_FRAC_OFFSET_MASK       0xfffff
+
+#define IPROC_CLK_ARM_DIV_OFFSET                     0xe00
+#define IPROC_CLK_ARM_DIV_PLL_SELECT_OVERRIDE_SHIFT  4
+#define IPROC_CLK_ARM_DIV_ARM_PLL_SELECT_MASK        0xf
+
+#define IPROC_CLK_POLICY_DBG_OFFSET                  0xec0
+#define IPROC_CLK_POLICY_DBG_ACT_FREQ_SHIFT          12
+#define IPROC_CLK_POLICY_DBG_ACT_FREQ_MASK           0x7
+
+enum iproc_arm_pll_fid {
+       ARM_PLL_FID_CRYSTAL_CLK   = 0,
+       ARM_PLL_FID_SYS_CLK       = 2,
+       ARM_PLL_FID_CH0_SLOW_CLK  = 6,
+       ARM_PLL_FID_CH1_FAST_CLK  = 7
+};
+
+struct iproc_arm_pll {
+       struct clk_hw hw;
+       void __iomem *base;
+       struct clk_onecell_data clk_data;
+       unsigned long rate;
+};
+
+#define to_iproc_arm_pll(hw) container_of(hw, struct iproc_arm_pll, hw)
+
+static unsigned int __get_fid(struct iproc_arm_pll *pll)
+{
+       u32 val;
+       unsigned int policy, fid, active_fid;
+
+       val = readl(pll->base + IPROC_CLK_ARM_DIV_OFFSET);
+       if (val & (1 << IPROC_CLK_ARM_DIV_PLL_SELECT_OVERRIDE_SHIFT))
+               policy = val & IPROC_CLK_ARM_DIV_ARM_PLL_SELECT_MASK;
+       else
+               policy = 0;
+
+       /* something is seriously wrong */
+       BUG_ON(policy > IPROC_CLK_MAX_FREQ_POLICY);
+
+       val = readl(pll->base + IPROC_CLK_POLICY_FREQ_OFFSET);
+       fid = (val >> (IPROC_CLK_POLICY_FREQ_POLICY_FREQ_SHIFT * policy)) &
+               IPROC_CLK_POLICY_FREQ_POLICY_FREQ_MASK;
+
+       val = readl(pll->base + IPROC_CLK_POLICY_DBG_OFFSET);
+       active_fid = IPROC_CLK_POLICY_DBG_ACT_FREQ_MASK &
+               (val >> IPROC_CLK_POLICY_DBG_ACT_FREQ_SHIFT);
+       if (fid != active_fid) {
+               pr_debug("%s: fid override %u->%u\n", __func__, fid,
+                               active_fid);
+               fid = active_fid;
+       }
+
+       pr_debug("%s: active fid: %u\n", __func__, fid);
+
+       return fid;
+}
+
+/*
+ * Determine the mdiv (post divider) based on the frequency ID being used.
+ * There are 4 sources that can be used to derive the output clock rate:
+ *    - 25 MHz Crystal
+ *    - System clock
+ *    - PLL channel 0 (slow clock)
+ *    - PLL channel 1 (fast clock)
+ */
+static int __get_mdiv(struct iproc_arm_pll *pll)
+{
+       unsigned int fid;
+       int mdiv;
+       u32 val;
+
+       fid = __get_fid(pll);
+
+       switch (fid) {
+       case ARM_PLL_FID_CRYSTAL_CLK:
+       case ARM_PLL_FID_SYS_CLK:
+               mdiv = 1;
+               break;
+
+       case ARM_PLL_FID_CH0_SLOW_CLK:
+               val = readl(pll->base + IPROC_CLK_PLLARMC_OFFSET);
+               mdiv = val & IPROC_CLK_PLLARMC_MDIV_MASK;
+               if (mdiv == 0)
+                       mdiv = 256;
+               break;
+
+       case ARM_PLL_FID_CH1_FAST_CLK:
+               val = readl(pll->base + IPROC_CLK_PLLARMCTL5_OFFSET);
+               mdiv = val & IPROC_CLK_PLLARMCTL5_H_MDIV_MASK;
+               if (mdiv == 0)
+                       mdiv = 256;
+               break;
+
+       default:
+               mdiv = -EFAULT;
+       }
+
+       return mdiv;
+}
+
+static unsigned int __get_ndiv(struct iproc_arm_pll *pll)
+{
+       u32 val;
+       unsigned int ndiv_int, ndiv_frac, ndiv;
+
+       val = readl(pll->base + IPROC_CLK_PLLARM_OFFSET_OFFSET);
+       if (val & (1 << IPROC_CLK_PLLARM_SW_CTL_SHIFT)) {
+               /*
+                * offset mode is active. Read the ndiv from the PLLARM OFFSET
+                * register
+                */
+               ndiv_int = (val >> IPROC_CLK_PLLARM_NDIV_INT_OFFSET_SHIFT) &
+                       IPROC_CLK_PLLARM_NDIV_INT_OFFSET_MASK;
+               if (ndiv_int == 0)
+                       ndiv_int = 256;
+
+               ndiv_frac = val & IPROC_CLK_PLLARM_NDIV_FRAC_OFFSET_MASK;
+       } else {
+               /* offset mode not active */
+               val = readl(pll->base + IPROC_CLK_PLLARMA_OFFSET);
+               ndiv_int = (val >> IPROC_CLK_PLLARMA_NDIV_INT_SHIFT) &
+                       IPROC_CLK_PLLARMA_NDIV_INT_MASK;
+               if (ndiv_int == 0)
+                       ndiv_int = 1024;
+
+               val = readl(pll->base + IPROC_CLK_PLLARMB_OFFSET);
+               ndiv_frac = val & IPROC_CLK_PLLARMB_NDIV_FRAC_MASK;
+       }
+
+       ndiv = (ndiv_int << 20) | ndiv_frac;
+
+       return ndiv;
+}
+
+/*
+ * The output frequency of the ARM PLL is calculated based on the ARM PLL
+ * divider values:
+ *   pdiv = ARM PLL pre-divider
+ *   ndiv = ARM PLL multiplier
+ *   mdiv = ARM PLL post divider
+ *
+ * The frequency is calculated by:
+ *   ((ndiv * parent clock rate) / pdiv) / mdiv
+ */
+static unsigned long iproc_arm_pll_recalc_rate(struct clk_hw *hw,
+               unsigned long parent_rate)
+{
+       struct iproc_arm_pll *pll = to_iproc_arm_pll(hw);
+       u32 val;
+       int mdiv;
+       u64 ndiv;
+       unsigned int pdiv;
+
+       /* in bypass mode, use parent rate */
+       val = readl(pll->base + IPROC_CLK_PLLARMC_OFFSET);
+       if (val & (1 << IPROC_CLK_PLLARMC_BYPCLK_EN_SHIFT)) {
+               pll->rate = parent_rate;
+               return pll->rate;
+       }
+
+       /* PLL needs to be locked */
+       val = readl(pll->base + IPROC_CLK_PLLARMA_OFFSET);
+       if (!(val & (1 << IPROC_CLK_PLLARMA_LOCK_SHIFT))) {
+               pll->rate = 0;
+               return 0;
+       }
+
+       pdiv = (val >> IPROC_CLK_PLLARMA_PDIV_SHIFT) &
+               IPROC_CLK_PLLARMA_PDIV_MASK;
+       if (pdiv == 0)
+               pdiv = 16;
+
+       ndiv = __get_ndiv(pll);
+       mdiv = __get_mdiv(pll);
+       if (mdiv <= 0) {
+               pll->rate = 0;
+               return 0;
+       }
+       pll->rate = (ndiv * parent_rate) >> 20;
+       pll->rate = (pll->rate / pdiv) / mdiv;
+
+       pr_debug("%s: ARM PLL rate: %lu. parent rate: %lu\n", __func__,
+                       pll->rate, parent_rate);
+       pr_debug("%s: ndiv_int: %u, pdiv: %u, mdiv: %d\n", __func__,
+                       (unsigned int)(ndiv >> 20), pdiv, mdiv);
+
+       return pll->rate;
+}
+
+static const struct clk_ops iproc_arm_pll_ops = {
+       .recalc_rate = iproc_arm_pll_recalc_rate,
+};
+
+void __init iproc_armpll_setup(struct device_node *node)
+{
+       int ret;
+       struct clk *clk;
+       struct iproc_arm_pll *pll;
+       struct clk_init_data init;
+       const char *parent_name;
+
+       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+       if (WARN_ON(!pll))
+               return;
+
+       pll->base = of_iomap(node, 0);
+       if (WARN_ON(!pll->base))
+               goto err_free_pll;
+
+       init.name = node->name;
+       init.ops = &iproc_arm_pll_ops;
+       init.flags = 0;
+       parent_name = of_clk_get_parent_name(node, 0);
+       init.parent_names = (parent_name ? &parent_name : NULL);
+       init.num_parents = (parent_name ? 1 : 0);
+       pll->hw.init = &init;
+
+       clk = clk_register(NULL, &pll->hw);
+       if (WARN_ON(IS_ERR(clk)))
+               goto err_iounmap;
+
+       pll->clk_data.clk_num = 1;
+       pll->clk_data.clks = &clk;
+
+       ret = of_clk_add_provider(node, of_clk_src_onecell_get, &pll->clk_data);
+       if (WARN_ON(ret))
+               goto err_clk_unregister;
+
+       return;
+
+err_clk_unregister:
+       clk_unregister(clk);
+err_iounmap:
+       iounmap(pll->base);
+err_free_pll:
+       kfree(pll);
+}

drivers/clk/bcm/clk-iproc-asiu.c new file +275 -0

new file mode 100644
index 0000000..ab86b8c
@@ -0,0 +1,275 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+
+#include "clk-iproc.h"
+
+struct iproc_asiu;
+
+struct iproc_asiu_clk {
+       struct clk_hw hw;
+       const char *name;
+       struct iproc_asiu *asiu;
+       unsigned long rate;
+       struct iproc_asiu_div div;
+       struct iproc_asiu_gate gate;
+};
+
+struct iproc_asiu {
+       void __iomem *div_base;
+       void __iomem *gate_base;
+
+       struct clk_onecell_data clk_data;
+       struct iproc_asiu_clk *clks;
+};
+
+#define to_asiu_clk(hw) container_of(hw, struct iproc_asiu_clk, hw)
+
+static int iproc_asiu_clk_enable(struct clk_hw *hw)
+{
+       struct iproc_asiu_clk *clk = to_asiu_clk(hw);
+       struct iproc_asiu *asiu = clk->asiu;
+       u32 val;
+
+       /* some clocks at the ASIU level are always enabled */
+       if (clk->gate.offset == IPROC_CLK_INVALID_OFFSET)
+               return 0;
+
+       val = readl(asiu->gate_base + clk->gate.offset);
+       val |= (1 << clk->gate.en_shift);
+       writel(val, asiu->gate_base + clk->gate.offset);
+
+       return 0;
+}
+
+static void iproc_asiu_clk_disable(struct clk_hw *hw)
+{
+       struct iproc_asiu_clk *clk = to_asiu_clk(hw);
+       struct iproc_asiu *asiu = clk->asiu;
+       u32 val;
+
+       /* some clocks at the ASIU level are always enabled */
+       if (clk->gate.offset == IPROC_CLK_INVALID_OFFSET)
+               return;
+
+       val = readl(asiu->gate_base + clk->gate.offset);
+       val &= ~(1 << clk->gate.en_shift);
+       writel(val, asiu->gate_base + clk->gate.offset);
+}
+
+static unsigned long iproc_asiu_clk_recalc_rate(struct clk_hw *hw,
+               unsigned long parent_rate)
+{
+       struct iproc_asiu_clk *clk = to_asiu_clk(hw);
+       struct iproc_asiu *asiu = clk->asiu;
+       u32 val;
+       unsigned int div_h, div_l;
+
+       if (parent_rate == 0) {
+               clk->rate = 0;
+               return 0;
+       }
+
+       /* if clock divisor is not enabled, simply return parent rate */
+       val = readl(asiu->div_base + clk->div.offset);
+       if ((val & (1 << clk->div.en_shift)) == 0) {
+               clk->rate = parent_rate;
+               return parent_rate;
+       }
+
+       /* clock rate = parent rate / (high_div + 1) + (low_div + 1) */
+       div_h = (val >> clk->div.high_shift) & bit_mask(clk->div.high_width);
+       div_h++;
+       div_l = (val >> clk->div.low_shift) & bit_mask(clk->div.low_width);
+       div_l++;
+
+       clk->rate = parent_rate / (div_h + div_l);
+       pr_debug("%s: rate: %lu. parent rate: %lu div_h: %u div_l: %u\n",
+               __func__, clk->rate, parent_rate, div_h, div_l);
+
+       return clk->rate;
+}
+
+static long iproc_asiu_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+               unsigned long *parent_rate)
+{
+       unsigned int div;
+
+       if (rate == 0 || *parent_rate == 0)
+               return -EINVAL;
+
+       if (rate == *parent_rate)
+               return *parent_rate;
+
+       div = DIV_ROUND_UP(*parent_rate, rate);
+       if (div < 2)
+               return *parent_rate;
+
+       return *parent_rate / div;
+}
+
+static int iproc_asiu_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+               unsigned long parent_rate)
+{
+       struct iproc_asiu_clk *clk = to_asiu_clk(hw);
+       struct iproc_asiu *asiu = clk->asiu;
+       unsigned int div, div_h, div_l;
+       u32 val;
+
+       if (rate == 0 || parent_rate == 0)
+               return -EINVAL;
+
+       /* simply disable the divisor if one wants the same rate as parent */
+       if (rate == parent_rate) {
+               val = readl(asiu->div_base + clk->div.offset);
+               val &= ~(1 << clk->div.en_shift);
+               writel(val, asiu->div_base + clk->div.offset);
+               return 0;
+       }
+
+       div = DIV_ROUND_UP(parent_rate, rate);
+       if (div < 2)
+               return -EINVAL;
+
+       div_h = div_l = div >> 1;
+       div_h--;
+       div_l--;
+
+       val = readl(asiu->div_base + clk->div.offset);
+       val |= 1 << clk->div.en_shift;
+       if (div_h) {
+               val &= ~(bit_mask(clk->div.high_width)
+                               << clk->div.high_shift);
+               val |= div_h << clk->div.high_shift;
+       } else {
+               val &= ~(bit_mask(clk->div.high_width)
+                               << clk->div.high_shift);
+       }
+       if (div_l) {
+               val &= ~(bit_mask(clk->div.low_width) << clk->div.low_shift);
+               val |= div_l << clk->div.low_shift;
+       } else {
+               val &= ~(bit_mask(clk->div.low_width) << clk->div.low_shift);
+       }
+       writel(val, asiu->div_base + clk->div.offset);
+
+       return 0;
+}
+
+static const struct clk_ops iproc_asiu_ops = {
+       .enable = iproc_asiu_clk_enable,
+       .disable = iproc_asiu_clk_disable,
+       .recalc_rate = iproc_asiu_clk_recalc_rate,
+       .round_rate = iproc_asiu_clk_round_rate,
+       .set_rate = iproc_asiu_clk_set_rate,
+};
+
+void __init iproc_asiu_setup(struct device_node *node,
+               const struct iproc_asiu_div *div,
+               const struct iproc_asiu_gate *gate, unsigned int num_clks)
+{
+       int i, ret;
+       struct iproc_asiu *asiu;
+
+       if (WARN_ON(!gate || !div))
+               return;
+
+       asiu = kzalloc(sizeof(*asiu), GFP_KERNEL);
+       if (WARN_ON(!asiu))
+               return;
+
+       asiu->clk_data.clk_num = num_clks;
+       asiu->clk_data.clks = kcalloc(num_clks, sizeof(*asiu->clk_data.clks),
+                       GFP_KERNEL);
+       if (WARN_ON(!asiu->clk_data.clks))
+               goto err_clks;
+
+       asiu->clks = kcalloc(num_clks, sizeof(*asiu->clks), GFP_KERNEL);
+       if (WARN_ON(!asiu->clks))
+               goto err_asiu_clks;
+
+       asiu->div_base = of_iomap(node, 0);
+       if (WARN_ON(!asiu->div_base))
+               goto err_iomap_div;
+
+       asiu->gate_base = of_iomap(node, 1);
+       if (WARN_ON(!asiu->gate_base))
+               goto err_iomap_gate;
+
+       for (i = 0; i < num_clks; i++) {
+               struct clk_init_data init;
+               struct clk *clk;
+               const char *parent_name;
+               struct iproc_asiu_clk *asiu_clk;
+               const char *clk_name;
+
+               clk_name = kzalloc(IPROC_CLK_NAME_LEN, GFP_KERNEL);
+               if (WARN_ON(!clk_name))
+                       goto err_clk_register;
+
+               ret = of_property_read_string_index(node, "clock-output-names",
+                               i, &clk_name);
+               if (WARN_ON(ret))
+                       goto err_clk_register;
+
+               asiu_clk = &asiu->clks[i];
+               asiu_clk->name = clk_name;
+               asiu_clk->asiu = asiu;
+               asiu_clk->div = div[i];
+               asiu_clk->gate = gate[i];
+               init.name = clk_name;
+               init.ops = &iproc_asiu_ops;
+               init.flags = 0;
+               parent_name = of_clk_get_parent_name(node, 0);
+               init.parent_names = (parent_name ? &parent_name : NULL);
+               init.num_parents = (parent_name ? 1 : 0);
+               asiu_clk->hw.init = &init;
+
+               clk = clk_register(NULL, &asiu_clk->hw);
+               if (WARN_ON(IS_ERR(clk)))
+                       goto err_clk_register;
+               asiu->clk_data.clks[i] = clk;
+       }
+
+       ret = of_clk_add_provider(node, of_clk_src_onecell_get,
+                       &asiu->clk_data);
+       if (WARN_ON(ret))
+               goto err_clk_register;
+
+       return;
+
+err_clk_register:
+       for (i = 0; i < num_clks; i++)
+               kfree(asiu->clks[i].name);
+       iounmap(asiu->gate_base);
+
+err_iomap_gate:
+       iounmap(asiu->div_base);
+
+err_iomap_div:
+       kfree(asiu->clks);
+
+err_asiu_clks:
+       kfree(asiu->clk_data.clks);
+
+err_clks:
+       kfree(asiu);
+}

drivers/clk/bcm/clk-iproc-clk.c new file +238 -0

new file mode 100644
index 0000000..be3c42c
@@ -0,0 +1,238 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+
+#include "clk-iproc.h"
+
+struct iproc_pll;
+
+struct iproc_clk {
+       struct clk_hw hw;
+       const char *name;
+       struct iproc_pll *pll;
+       unsigned long rate;
+       const struct iproc_clk_ctrl *ctrl;
+};
+
+struct iproc_pll {
+       void __iomem *base;
+       struct clk_onecell_data clk_data;
+       struct iproc_clk *clks;
+};
+
+#define to_iproc_clk(hw) container_of(hw, struct iproc_clk, hw)
+
+static int iproc_clk_enable(struct clk_hw *hw)
+{
+       struct iproc_clk *clk = to_iproc_clk(hw);
+       const struct iproc_clk_ctrl *ctrl = clk->ctrl;
+       struct iproc_pll *pll = clk->pll;
+       u32 val;
+
+       /* channel enable is active low */
+       val = readl(pll->base + ctrl->enable.offset);
+       val &= ~(1 << ctrl->enable.enable_shift);
+       writel(val, pll->base + ctrl->enable.offset);
+
+       /* also make sure channel is not held */
+       val = readl(pll->base + ctrl->enable.offset);
+       val &= ~(1 << ctrl->enable.hold_shift);
+       writel(val, pll->base + ctrl->enable.offset);
+
+       return 0;
+}
+
+static void iproc_clk_disable(struct clk_hw *hw)
+{
+       struct iproc_clk *clk = to_iproc_clk(hw);
+       const struct iproc_clk_ctrl *ctrl = clk->ctrl;
+       struct iproc_pll *pll = clk->pll;
+       u32 val;
+
+       if (ctrl->flags & IPROC_CLK_AON)
+               return;
+
+       val = readl(pll->base + ctrl->enable.offset);
+       val |= 1 << ctrl->enable.enable_shift;
+       writel(val, pll->base + ctrl->enable.offset);
+}
+
+static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
+               unsigned long parent_rate)
+{
+       struct iproc_clk *clk = to_iproc_clk(hw);
+       const struct iproc_clk_ctrl *ctrl = clk->ctrl;
+       struct iproc_pll *pll = clk->pll;
+       u32 val;
+       unsigned int mdiv;
+
+       if (parent_rate == 0)
+               return 0;
+
+       val = readl(pll->base + ctrl->mdiv.offset);
+       mdiv = (val >> ctrl->mdiv.shift) & bit_mask(ctrl->mdiv.width);
+       if (mdiv == 0)
+               mdiv = 256;
+
+       clk->rate = parent_rate / mdiv;
+
+       return clk->rate;
+}
+
+static long iproc_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+               unsigned long *parent_rate)
+{
+       unsigned int div;
+
+       if (rate == 0 || *parent_rate == 0)
+               return -EINVAL;
+
+       if (rate == *parent_rate)
+               return *parent_rate;
+
+       div = DIV_ROUND_UP(*parent_rate, rate);
+       if (div < 2)
+               return *parent_rate;
+
+       if (div > 256)
+               div = 256;
+
+       return *parent_rate / div;
+}
+
+static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+               unsigned long parent_rate)
+{
+       struct iproc_clk *clk = to_iproc_clk(hw);
+       const struct iproc_clk_ctrl *ctrl = clk->ctrl;
+       struct iproc_pll *pll = clk->pll;
+       u32 val;
+       unsigned int div;
+
+       if (rate == 0 || parent_rate == 0)
+               return -EINVAL;
+
+       div = DIV_ROUND_UP(parent_rate, rate);
+       if (div > 256)
+               return -EINVAL;
+
+       val = readl(pll->base + ctrl->mdiv.offset);
+       if (div == 256) {
+               val &= ~(bit_mask(ctrl->mdiv.width) << ctrl->mdiv.shift);
+       } else {
+               val &= ~(bit_mask(ctrl->mdiv.width) << ctrl->mdiv.shift);
+               val |= div << ctrl->mdiv.shift;
+       }
+       writel(val, pll->base + ctrl->mdiv.offset);
+       clk->rate = parent_rate / div;
+
+       return 0;
+}
+
+static const struct clk_ops iproc_clk_ops = {
+       .enable = iproc_clk_enable,
+       .disable = iproc_clk_disable,
+       .recalc_rate = iproc_clk_recalc_rate,
+       .round_rate = iproc_clk_round_rate,
+       .set_rate = iproc_clk_set_rate,
+};
+
+void __init iproc_clk_setup(struct device_node *node,
+               const struct iproc_clk_ctrl *ctrl, unsigned int num_clks)
+{
+       int i, ret;
+       struct iproc_pll *pll;
+
+       if (WARN_ON(!ctrl))
+               return;
+
+       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+       if (WARN_ON(!pll))
+               return;
+
+       pll->clk_data.clk_num = num_clks;
+       pll->clk_data.clks = kcalloc(num_clks, sizeof(*pll->clk_data.clks),
+                       GFP_KERNEL);
+       if (WARN_ON(!pll->clk_data.clks))
+               goto err_clks;
+
+       pll->clks = kcalloc(num_clks, sizeof(*pll->clks), GFP_KERNEL);
+       if (WARN_ON(!pll->clks))
+               goto err_pll_clks;
+
+       pll->base = of_iomap(node, 0);
+       if (WARN_ON(!pll->base))
+               goto err_iomap;
+
+       for (i = 0; i < num_clks; i++) {
+               struct clk_init_data init;
+               struct clk *clk;
+               const char *parent_name;
+               struct iproc_clk *iclk;
+               const char *clk_name;
+
+               clk_name = kzalloc(IPROC_CLK_NAME_LEN, GFP_KERNEL);
+               if (WARN_ON(!clk_name))
+                       goto err_clk_register;
+
+               ret = of_property_read_string_index(node, "clock-output-names",
+                               i, &clk_name);
+               if (WARN_ON(ret))
+                       goto err_clk_register;
+
+               iclk = &pll->clks[i];
+               iclk->name = clk_name;
+               iclk->pll = pll;
+               iclk->ctrl = &ctrl[i];
+               init.name = clk_name;
+               init.ops = &iproc_clk_ops;
+               init.flags = 0;
+               parent_name = of_clk_get_parent_name(node, 0);
+               init.parent_names = (parent_name ? &parent_name : NULL);
+               init.num_parents = (parent_name ? 1 : 0);
+               iclk->hw.init = &init;
+
+               clk = clk_register(NULL, &iclk->hw);
+               if (WARN_ON(IS_ERR(clk)))
+                       goto err_clk_register;
+               pll->clk_data.clks[i] = clk;
+       }
+
+       ret = of_clk_add_provider(node, of_clk_src_onecell_get, &pll->clk_data);
+       if (WARN_ON(ret))
+               goto err_clk_register;
+
+       return;
+
+err_clk_register:
+       for (i = 0; i < num_clks; i++)
+               kfree(pll->clks[i].name);
+       iounmap(pll->base);
+
+err_iomap:
+       kfree(pll->clks);
+
+err_pll_clks:
+       kfree(pll->clk_data.clks);
+
+err_clks:
+       kfree(pll);
+}

drivers/clk/bcm/clk-iproc-pll.c new file +483 -0

new file mode 100644
index 0000000..cd3bd38
@@ -0,0 +1,483 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+
+#include "clk-iproc.h"
+
+#define PLL_VCO_HIGH_SHIFT 19
+#define PLL_VCO_LOW_SHIFT  30
+
+/* number of delay loops waiting for PLL to lock */
+#define LOCK_DELAY 100
+
+/* number of VCO frequency bands */
+#define NUM_FREQ_BANDS 8
+
+#define NUM_KP_BANDS 3
+enum kp_band {
+       KP_BAND_MID = 0,
+       KP_BAND_HIGH,
+       KP_BAND_HIGH_HIGH
+};
+
+static const unsigned int kp_table[NUM_KP_BANDS][NUM_FREQ_BANDS] = {
+       { 5, 6, 6, 7, 7, 8, 9, 10 },
+       { 4, 4, 5, 5, 6, 7, 8, 9  },
+       { 4, 5, 5, 6, 7, 8, 9, 10 },
+};
+
+static const unsigned long ref_freq_table[NUM_FREQ_BANDS][2] = {
+       { 10000000,  12500000  },
+       { 12500000,  15000000  },
+       { 15000000,  20000000  },
+       { 20000000,  25000000  },
+       { 25000000,  50000000  },
+       { 50000000,  75000000  },
+       { 75000000,  100000000 },
+       { 100000000, 125000000 },
+};
+
+enum vco_freq_range {
+       VCO_LOW       = 700000000U,
+       VCO_MID       = 1200000000U,
+       VCO_HIGH      = 2200000000U,
+       VCO_HIGH_HIGH = 3100000000U,
+       VCO_MAX       = 4000000000U,
+};
+
+struct iproc_pll {
+       struct clk_hw hw;
+       void __iomem *pll_base;
+       void __iomem *pwr_base;
+       void __iomem *asiu_base;
+       struct clk_onecell_data clk_data;
+       const char *name;
+       const struct iproc_pll_ctrl *ctrl;
+       const struct iproc_pll_vco_freq_param *vco_param;
+       unsigned int num_vco_entries;
+       unsigned long rate;
+};
+
+#define to_iproc_pll(hw) container_of(hw, struct iproc_pll, hw)
+
+/*
+ * Get the clock rate based on name
+ */
+static unsigned long __get_rate(const char *clk_name)
+{
+       struct clk *clk;
+
+       clk = __clk_lookup(clk_name);
+       if (!clk) {
+               pr_err("%s: unable to find clock by name: %s\n", __func__,
+                               clk_name);
+               return 0;
+       }
+
+       return clk_get_rate(clk);
+}
+
+/*
+ * Based on the target frequency, find a match from the VCO frequency parameter
+ * table and return its index
+ */
+static int pll_get_rate_index(struct iproc_pll *pll, unsigned int target_rate)
+{
+       int i;
+
+       for (i = 0; i < pll->num_vco_entries; i++)
+               if (target_rate == pll->vco_param[i].rate)
+                       break;
+
+       if (i >= pll->num_vco_entries)
+               return -EINVAL;
+
+       return i;
+}
+
+static int get_kp(unsigned long ref_freq, enum kp_band kp_index)
+{
+       int i;
+
+       if (ref_freq < ref_freq_table[0][0])
+               return -EINVAL;
+
+       for (i = 0; i < NUM_FREQ_BANDS; i++) {
+               if (ref_freq >= ref_freq_table[i][0] &&
+                       ref_freq < ref_freq_table[i][1])
+                       return kp_table[kp_index][i];
+       }
+       return -EINVAL;
+}
+
+static int pll_wait_for_lock(struct iproc_pll *pll)
+{
+       int i;
+       const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+
+       for (i = 0; i < LOCK_DELAY; i++) {
+               u32 val = readl(pll->pll_base + ctrl->status.offset);
+
+               if (val & (1 << ctrl->status.shift))
+                       return 0;
+               udelay(10);
+       }
+
+       return -EIO;
+}
+
+static void __pll_disable(struct iproc_pll *pll)
+{
+       const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+       u32 val;
+
+       if (ctrl->flags & IPROC_CLK_PLL_ASIU) {
+               val = readl(pll->asiu_base + ctrl->asiu.offset);
+               val &= ~(1 << ctrl->asiu.en_shift);
+               writel(val, pll->asiu_base + ctrl->asiu.offset);
+       }
+
+       /* latch input value so core power can be shut down */
+       val = readl(pll->pwr_base + ctrl->aon.offset);
+       val |= (1 << ctrl->aon.iso_shift);
+       writel(val, pll->pwr_base + ctrl->aon.offset);
+
+       /* power down the core */
+       val &= ~(bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
+       writel(val, pll->pwr_base + ctrl->aon.offset);
+}
+
+static int __pll_enable(struct iproc_pll *pll)
+{
+       const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+       u32 val;
+
+       /* power up the PLL and make sure it's not latched */
+       val = readl(pll->pwr_base + ctrl->aon.offset);
+       val |= bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift;
+       val &= ~(1 << ctrl->aon.iso_shift);
+       writel(val, pll->pwr_base + ctrl->aon.offset);
+
+       /* certain PLLs also need to be ungated from the ASIU top level */
+       if (ctrl->flags & IPROC_CLK_PLL_ASIU) {
+               val = readl(pll->asiu_base + ctrl->asiu.offset);
+               val |= (1 << ctrl->asiu.en_shift);
+               writel(val, pll->asiu_base + ctrl->asiu.offset);
+       }
+
+       return 0;
+}
+
+static void __pll_put_in_reset(struct iproc_pll *pll)
+{
+       u32 val;
+       const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+       const struct iproc_pll_reset_ctrl *reset = &ctrl->reset;
+
+       val = readl(pll->pll_base + reset->offset);
+       val &= ~(1 << reset->reset_shift | 1 << reset->p_reset_shift);
+       writel(val, pll->pll_base + reset->offset);
+}
+
+static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
+               unsigned int ka, unsigned int ki)
+{
+       u32 val;
+       const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+       const struct iproc_pll_reset_ctrl *reset = &ctrl->reset;
+
+       val = readl(pll->pll_base + reset->offset);
+       val &= ~(bit_mask(reset->ki_width) << reset->ki_shift |
+               bit_mask(reset->kp_width) << reset->kp_shift |
+               bit_mask(reset->ka_width) << reset->ka_shift);
+       val |=  ki << reset->ki_shift | kp << reset->kp_shift |
+               ka << reset->ka_shift;
+       val |= 1 << reset->reset_shift | 1 << reset->p_reset_shift;
+       writel(val, pll->pll_base + reset->offset);
+}
+
+static int pll_set_rate(struct iproc_pll *pll, unsigned int rate_index,
+               unsigned long parent_rate)
+{
+       const struct iproc_pll_vco_freq_param *vco =
+                               &pll->vco_param[rate_index];
+       const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+       int ka = 0, ki, kp, ret;
+       unsigned long rate = vco->rate;
+       u32 val;
+       enum kp_band kp_index;
+       unsigned long ref_freq;
+
+       /*
+        * reference frequency = parent frequency / PDIV
+        * If PDIV = 0, then it becomes a multiplier (x2)
+        */
+       if (vco->pdiv == 0)
+               ref_freq = parent_rate * 2;
+       else
+               ref_freq = parent_rate / vco->pdiv;
+
+       /* determine Ki and Kp index based on target VCO frequency */
+       if (rate >= VCO_LOW && rate < VCO_HIGH) {
+               ki = 4;
+               kp_index = KP_BAND_MID;
+       } else if (rate >= VCO_HIGH && rate && rate < VCO_HIGH_HIGH) {
+               ki = 3;
+               kp_index = KP_BAND_HIGH;
+       } else if (rate >= VCO_HIGH_HIGH && rate < VCO_MAX) {
+               ki = 3;
+               kp_index = KP_BAND_HIGH_HIGH;
+       } else {
+               pr_err("%s: pll: %s has invalid rate: %lu\n", __func__,
+                               pll->name, rate);
+               return -EINVAL;
+       }
+
+       kp = get_kp(ref_freq, kp_index);
+       if (kp < 0) {
+               pr_err("%s: pll: %s has invalid kp\n", __func__, pll->name);
+               return kp;
+       }
+
+       ret = __pll_enable(pll);
+       if (ret) {
+               pr_err("%s: pll: %s fails to enable\n", __func__, pll->name);
+               return ret;
+       }
+
+       /* put PLL in reset */
+       __pll_put_in_reset(pll);
+
+       writel(0, pll->pll_base + ctrl->vco_ctrl.u_offset);
+       val = readl(pll->pll_base + ctrl->vco_ctrl.l_offset);
+
+       if (rate >= VCO_LOW && rate < VCO_MID)
+               val |= (1 << PLL_VCO_LOW_SHIFT);
+
+       if (rate < VCO_HIGH)
+               val &= ~(1 << PLL_VCO_HIGH_SHIFT);
+       else
+               val |= (1 << PLL_VCO_HIGH_SHIFT);
+
+       writel(val, pll->pll_base + ctrl->vco_ctrl.l_offset);
+
+       /* program integer part of NDIV */
+       val = readl(pll->pll_base + ctrl->ndiv_int.offset);
+       val &= ~(bit_mask(ctrl->ndiv_int.width) << ctrl->ndiv_int.shift);
+       val |= vco->ndiv_int << ctrl->ndiv_int.shift;
+       writel(val, pll->pll_base + ctrl->ndiv_int.offset);
+
+       /* program fractional part of NDIV */
+       if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
+               val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
+               val &= ~(bit_mask(ctrl->ndiv_frac.width) <<
+                               ctrl->ndiv_frac.shift);
+               val |= vco->ndiv_frac << ctrl->ndiv_frac.shift;
+               writel(val, pll->pll_base + ctrl->ndiv_frac.offset);
+       }
+
+       /* program PDIV */
+       val = readl(pll->pll_base + ctrl->pdiv.offset);
+       val &= ~(bit_mask(ctrl->pdiv.width) << ctrl->pdiv.shift);
+       val |= vco->pdiv << ctrl->pdiv.shift;
+       writel(val, pll->pll_base + ctrl->pdiv.offset);
+
+       __pll_bring_out_reset(pll, kp, ka, ki);
+
+       ret = pll_wait_for_lock(pll);
+       if (ret < 0) {
+               pr_err("%s: pll: %s failed to lock\n", __func__, pll->name);
+               return ret;
+       }
+
+       return 0;
+}
+
+static int iproc_pll_enable(struct clk_hw *hw)
+{
+       struct iproc_pll *pll = to_iproc_pll(hw);
+
+       return __pll_enable(pll);
+}
+
+static void iproc_pll_disable(struct clk_hw *hw)
+{
+       struct iproc_pll *pll = to_iproc_pll(hw);
+       const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+
+       if (ctrl->flags & IPROC_CLK_AON)
+               return;
+
+       __pll_disable(pll);
+}
+
+static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
+       unsigned long parent_rate)
+{
+       struct iproc_pll *pll = to_iproc_pll(hw);
+       const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+       u32 val;
+       u64 ndiv;
+       unsigned int ndiv_int, ndiv_frac, pdiv;
+
+       if (parent_rate == 0)
+               return 0;
+
+       /* PLL needs to be locked */
+       val = readl(pll->pll_base + ctrl->status.offset);
+       if ((val & (1 << ctrl->status.shift)) == 0) {
+               pll->rate = 0;
+               return 0;
+       }
+
+       /*
+        * PLL output frequency =
+        *
+        * ((ndiv_int + ndiv_frac / 2^20) * (parent clock rate / pdiv)
+        */
+       val = readl(pll->pll_base + ctrl->ndiv_int.offset);
+       ndiv_int = (val >> ctrl->ndiv_int.shift) &
+               bit_mask(ctrl->ndiv_int.width);
+       ndiv = ndiv_int << ctrl->ndiv_int.shift;
+
+       if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
+               val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
+               ndiv_frac = (val >> ctrl->ndiv_frac.shift) &
+                       bit_mask(ctrl->ndiv_frac.width);
+
+               if (ndiv_frac != 0)
+                       ndiv = (ndiv_int << ctrl->ndiv_int.shift) | ndiv_frac;
+       }
+
+       val = readl(pll->pll_base + ctrl->pdiv.offset);
+       pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
+
+       pll->rate = (ndiv * parent_rate) >> ctrl->ndiv_int.shift;
+
+       if (pdiv == 0)
+               pll->rate *= 2;
+       else
+               pll->rate /= pdiv;
+
+       return pll->rate;
+}
+
+static const struct clk_ops iproc_pll_ops = {
+       .enable = iproc_pll_enable,
+       .disable = iproc_pll_disable,
+       .recalc_rate = iproc_pll_recalc_rate,
+};
+
+void __init iproc_pll_setup(struct device_node *node,
+               const struct iproc_pll_ctrl *ctrl,
+               const struct iproc_pll_vco_freq_param *vco_param,
+               unsigned int num_vco_entries)
+{
+       int ret;
+       struct clk *clk;
+       struct iproc_pll *pll;
+       struct clk_init_data init;
+       const char *parent_name;
+       unsigned int rate;
+
+       if (WARN_ON(!ctrl))
+               return;
+
+       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+       if (WARN_ON(!pll))
+               return;
+
+       pll->pll_base = of_iomap(node, 0);
+       if (WARN_ON(!pll->pll_base))
+               goto err_pll_iomap;
+
+       pll->pwr_base = of_iomap(node, 1);
+       if (WARN_ON(!pll->pwr_base))
+               goto err_pwr_iomap;
+
+       /* some PLLs require gating control at the top ASIU level */
+       if (ctrl->flags & IPROC_CLK_PLL_ASIU) {
+               pll->asiu_base = of_iomap(node, 2);
+               if (WARN_ON(!pll->asiu_base))
+                       goto err_asiu_iomap;
+       }
+
+       pll->ctrl = ctrl;
+       pll->name = node->name;
+       init.name = node->name;
+       init.ops = &iproc_pll_ops;
+       init.flags = 0;
+       parent_name = of_clk_get_parent_name(node, 0);
+       init.parent_names = (parent_name ? &parent_name : NULL);
+       init.num_parents = (parent_name ? 1 : 0);
+       pll->hw.init = &init;
+
+       /* configure the PLL to the desired VCO frequency if specified */
+       ret = of_property_read_u32(node, "clock-frequency", &rate);
+       if (!ret) {
+               unsigned long parent_rate;
+               int rate_index;
+
+               if (WARN_ON(!vco_param))
+                       goto err_clk_register;
+
+               pll->num_vco_entries = num_vco_entries;
+               pll->vco_param = vco_param;
+
+               parent_rate = __get_rate(parent_name);
+               if (WARN_ON(!parent_rate))
+                       goto err_clk_register;
+
+               rate_index = pll_get_rate_index(pll, rate);
+               if (WARN_ON(rate_index < 0))
+                       goto err_clk_register;
+
+               ret = pll_set_rate(pll, rate_index, parent_rate);
+               if (WARN_ON(ret))
+                       goto err_clk_register;
+       }
+
+       clk = clk_register(NULL, &pll->hw);
+       if (WARN_ON(IS_ERR(clk)))
+               goto err_clk_register;
+
+       pll->clk_data.clk_num = 1;
+       pll->clk_data.clks = &clk;
+
+       ret = of_clk_add_provider(node, of_clk_src_onecell_get,
+                       &pll->clk_data);
+       if (WARN_ON(ret))
+               goto err_clk_add;
+
+       return;
+
+err_clk_add:
+       clk_unregister(clk);
+err_clk_register:
+       if (pll->asiu_base)
+               iounmap(pll->asiu_base);
+err_asiu_iomap:
+       iounmap(pll->pwr_base);
+err_pwr_iomap:
+       iounmap(pll->pll_base);
+err_pll_iomap:
+       kfree(pll);
+}

drivers/clk/bcm/clk-iproc.h new file +155 -1

new file mode 100644
index 0000000..4aa0479
@@ -0,0 +1,155 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _CLK_IPROC_H
+#define _CLK_IPROC_H
+
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/clk-provider.h>
+
+#define IPROC_CLK_NAME_LEN 25
+#define IPROC_CLK_INVALID_OFFSET 0xffffffff
+#define bit_mask(width) ((1 << (width)) - 1)
+
+/* clock should not be disabled at runtime */
+#define IPROC_CLK_AON BIT(0)
+
+/* PLL requires gating through ASIU */
+#define IPROC_CLK_PLL_ASIU BIT(1)
+
+/* PLL has fractional part of the NDIV */
+#define IPROC_CLK_PLL_HAS_NDIV_FRAC BIT(2)
+
+/*
+ * Parameters for VCO frequency configuration
+ *
+ * VCO frequency =
+ * ((ndiv_int + ndiv_frac / 2^20) * (ref freqeuncy  / pdiv)
+ */
+struct iproc_pll_vco_freq_param {
+       unsigned long rate;
+       unsigned int ndiv_int;
+       unsigned int ndiv_frac;
+       unsigned int pdiv;
+};
+
+struct iproc_clk_reg_op {
+       unsigned int offset;
+       unsigned int shift;
+       unsigned int width;
+};
+
+/*
+ * Clock gating control at the top ASIU level
+ */
+struct iproc_asiu_gate {
+       unsigned int offset;
+       unsigned int en_shift;
+};
+
+/*
+ * Control of powering on/off of a PLL
+ *
+ * Before powering off a PLL, input isolation (ISO) needs to be enabled
+ */
+struct iproc_pll_aon_pwr_ctrl {
+       unsigned int offset;
+       unsigned int pwr_width;
+       unsigned int pwr_shift;
+       unsigned int iso_shift;
+};
+
+/*
+ * Control of the PLL reset, with Ki, Kp, and Ka parameters
+ */
+struct iproc_pll_reset_ctrl {
+       unsigned int offset;
+       unsigned int reset_shift;
+       unsigned int p_reset_shift;
+       unsigned int ki_shift;
+       unsigned int ki_width;
+       unsigned int kp_shift;
+       unsigned int kp_width;
+       unsigned int ka_shift;
+       unsigned int ka_width;
+};
+
+struct iproc_pll_vco_ctrl {
+       unsigned int u_offset;
+       unsigned int l_offset;
+};
+
+/*
+ * Main PLL control parameters
+ */
+struct iproc_pll_ctrl {
+       unsigned long flags;
+       struct iproc_pll_aon_pwr_ctrl aon;
+       struct iproc_asiu_gate asiu;
+       struct iproc_pll_reset_ctrl reset;
+       struct iproc_clk_reg_op ndiv_int;
+       struct iproc_clk_reg_op ndiv_frac;
+       struct iproc_clk_reg_op pdiv;
+       struct iproc_pll_vco_ctrl vco_ctrl;
+       struct iproc_clk_reg_op status;
+};
+
+/*
+ * Controls enabling/disabling a PLL derived clock
+ */
+struct iproc_clk_enable_ctrl {
+       unsigned int offset;
+       unsigned int enable_shift;
+       unsigned int hold_shift;
+       unsigned int bypass_shift;
+};
+
+/*
+ * Main clock control parameters for clocks derived from the PLLs
+ */
+struct iproc_clk_ctrl {
+       unsigned int channel;
+       unsigned long flags;
+       struct iproc_clk_enable_ctrl enable;
+       struct iproc_clk_reg_op mdiv;
+};
+
+/*
+ * Divisor of the ASIU clocks
+ */
+struct iproc_asiu_div {
+       unsigned int offset;
+       unsigned int en_shift;
+       unsigned int high_shift;
+       unsigned int high_width;
+       unsigned int low_shift;
+       unsigned int low_width;
+};
+
+extern void __init iproc_armpll_setup(struct device_node *node);
+extern void __init iproc_pll_setup(struct device_node *node,
+               const struct iproc_pll_ctrl *ctrl,
+               const struct iproc_pll_vco_freq_param *vco_param,
+               unsigned int num_freqs);
+extern void __init iproc_clk_setup(struct device_node *node,
+               const struct iproc_clk_ctrl *ctrl, unsigned int num_clks);
+extern void __init iproc_asiu_setup(struct device_node *node,
+               const struct iproc_asiu_div *div,
+               const struct iproc_asiu_gate *gate, unsigned int num_clks);
+
+#endif /* _CLK_IPROC_H */
--

1.7.9.5

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel