2013-08-07 15:53:57

by Mark Langsdorf

[permalink] [raw]
Subject: [PATCH v3 1/5] sata, highbank: fix ordering of SGPIO signals

The ACTIVITY and ERROR signals were reversed in the original commit.
Fix that so that hard drive activity does not show up on the error
light, and attempts to indicate that the hard drive is failing do
not show up as hard drive activity. This fixes a fairly serious
functional bug in the driver, but failing to apply this patch will
not cause any stability issues on the system.

Signed-off-by: Mark Langsdorf <[email protected]>
---
Changes from v2
Further rewords of the commit message.
Changes from v1
Expanded commit message explaining the problems with the
unpatched code.

drivers/ata/sata_highbank.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index d047d92..e9a4f46 100644
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -86,11 +86,11 @@ struct ecx_plat_data {

#define SGPIO_SIGNALS 3
#define ECX_ACTIVITY_BITS 0x300000
-#define ECX_ACTIVITY_SHIFT 2
+#define ECX_ACTIVITY_SHIFT 0
#define ECX_LOCATE_BITS 0x80000
#define ECX_LOCATE_SHIFT 1
#define ECX_FAULT_BITS 0x400000
-#define ECX_FAULT_SHIFT 0
+#define ECX_FAULT_SHIFT 2
static inline int sgpio_bit_shift(struct ecx_plat_data *pdata, u32 port,
u32 shift)
{
--
1.8.1.2


2013-08-07 15:53:58

by Mark Langsdorf

[permalink] [raw]
Subject: [PATCH v3 2/5] sata highbank: enable 64-bit DMA mask when using LPAE

From: Rob Herring <[email protected]>

Signed-off-by: Rob Herring <[email protected]>
Signed-off-by: Mark Langsdorf <[email protected]>
---
Changes from v1, v2
None.

drivers/ata/sata_highbank.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index e9a4f46..8b40025 100644
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -479,6 +479,9 @@ static int ahci_highbank_probe(struct platform_device *pdev)
if (hpriv->cap & HOST_CAP_PMP)
pi.flags |= ATA_FLAG_PMP;

+ if (hpriv->cap & HOST_CAP_64)
+ dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
+
/* CAP.NP sometimes indicate the index of the last enabled
* port, at other times, that of the last possible port, so
* determining the maximum port number requires looking at
--
1.8.1.2

2013-08-07 15:54:24

by Mark Langsdorf

[permalink] [raw]
Subject: [PATCH v3 5/5] sata, highbank: send extra clock cycles in SGPIO patterns

Some SGPIO PICs don't follow the standard very well and expect a certain
number of clock cycles or port frames in each SGPIO pattern. Add two
optional parameters in the DTB that can provide the number of extra
clock cycles to be sent before and after SGPIO pattern. Read those
parameters from the DTB and send the extra clock cycles.

Signed-off-by: Mark Langsdorf <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Changes from v2
None.
Changes from v1
Added an example to the bindings.
Forced the pre-clocks and post-clocks values to 0 if there is an
error while reading them or the values aren't in the DTB.

Documentation/devicetree/bindings/ata/sata_highbank.txt | 6 ++++++
drivers/ata/sata_highbank.c | 13 +++++++++++++
2 files changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/ata/sata_highbank.txt b/Documentation/devicetree/bindings/ata/sata_highbank.txt
index fdbd4476..6124a32 100644
--- a/Documentation/devicetree/bindings/ata/sata_highbank.txt
+++ b/Documentation/devicetree/bindings/ata/sata_highbank.txt
@@ -23,6 +23,10 @@ Optional properties:
- calxeda,tx-atten : a u32 array that contains TX attenuation override
codes, one per port. The upper 3 bytes are always
0 and thus ignored.
+- calxeda,pre-clocks : a u32 that indicates the number of additional clock
+ cycles to transmit before sending an SGPIO pattern
+- calxeda,post-clocks: a u32 that indicates the number of additional clock
+ cycles to transmit after sending an SGPIO pattern

Example:
sata@ffe08000 {
@@ -32,4 +36,6 @@ Example:
calxeda,port-phys = <&combophy5 0 &combophy0 0 &combophy0 1
&combophy0 2 &combophy0 3>;
calxeda,tx-atten = <0xff 22 0xff 0xff 23>;
+ calxeda,pre-clocks = <10>;
+ calxeda,post-clocks = <0>;
};
diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index a7c8038..7f5e5d9 100644
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -84,6 +84,9 @@ static DEFINE_SPINLOCK(sgpio_lock);

struct ecx_plat_data {
u32 n_ports;
+ /* number of extra clocks that the SGPIO PIC controller expects */
+ u32 pre_clocks;
+ u32 post_clocks;
unsigned sgpio_gpio[SGPIO_PINS];
u32 sgpio_pattern;
u32 port_to_sgpio[SGPIO_PORTS];
@@ -160,6 +163,9 @@ static ssize_t ecx_transmit_led_message(struct ata_port *ap, u32 state,
spin_lock_irqsave(&sgpio_lock, flags);
ecx_parse_sgpio(pdata, ap->port_no, state);
sgpio_out = pdata->sgpio_pattern;
+ for (i = 0; i < pdata->pre_clocks; i++)
+ ecx_led_cycle_clock(pdata);
+
gpio_set_value(pdata->sgpio_gpio[SLOAD], 1);
ecx_led_cycle_clock(pdata);
gpio_set_value(pdata->sgpio_gpio[SLOAD], 0);
@@ -172,6 +178,8 @@ static ssize_t ecx_transmit_led_message(struct ata_port *ap, u32 state,
sgpio_out >>= 1;
ecx_led_cycle_clock(pdata);
}
+ for (i = 0; i < pdata->post_clocks; i++)
+ ecx_led_cycle_clock(pdata);

/* save off new led state for port/slot */
emp->led_state = state;
@@ -206,6 +214,11 @@ static void highbank_set_em_messages(struct device *dev,
of_property_read_u32_array(np, "calxeda,led-order",
pdata->port_to_sgpio,
pdata->n_ports);
+ if (of_property_read_u32(np, "calxeda,pre-clocks", &pdata->pre_clocks))
+ pdata->pre_clocks = 0;
+ if (of_property_read_u32(np, "calxeda,post-clocks",
+ &pdata->post_clocks))
+ pdata->post_clocks = 0;

/* store em_loc */
hpriv->em_loc = 0;
--
1.8.1.2

2013-08-07 15:54:54

by Mark Langsdorf

[permalink] [raw]
Subject: [PATCH v3 4/5] sata, highbank: set tx_atten override bits

Some board designs do not drive the SATA transmit lines within the
specification. The ECME can provide override settings, on a per board
basis, to bring the transmit lines within spec. Read those settings
from the DTB and program them in.

Signed-off-by: Mark Langsdorf <[email protected]>
---
Changes from v2
None.
Changes from v1
Clarified that the array is a u32 array.
Added an example in the bindings.

.../devicetree/bindings/ata/sata_highbank.txt | 5 +-
drivers/ata/sata_highbank.c | 58 +++++++++++++++++-----
2 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/ata/sata_highbank.txt b/Documentation/devicetree/bindings/ata/sata_highbank.txt
index 1ac6d3d..fdbd4476 100644
--- a/Documentation/devicetree/bindings/ata/sata_highbank.txt
+++ b/Documentation/devicetree/bindings/ata/sata_highbank.txt
@@ -20,6 +20,9 @@ Optional properties:
indicator lights using the indicated GPIOs
- calxeda,led-order : a u32 array that map port numbers to offsets within the
SGPIO bitstream.
+- calxeda,tx-atten : a u32 array that contains TX attenuation override
+ codes, one per port. The upper 3 bytes are always
+ 0 and thus ignored.

Example:
sata@ffe08000 {
@@ -28,5 +31,5 @@ Example:
interrupts = <115>;
calxeda,port-phys = <&combophy5 0 &combophy0 0 &combophy0 1
&combophy0 2 &combophy0 3>;
-
+ calxeda,tx-atten = <0xff 22 0xff 0xff 23>;
};
diff --git a/drivers/ata/sata_highbank.c b/drivers/ata/sata_highbank.c
index 8b40025..a7c8038 100644
--- a/drivers/ata/sata_highbank.c
+++ b/drivers/ata/sata_highbank.c
@@ -46,14 +46,19 @@
#define CR_BUSY 0x0001
#define CR_START 0x0001
#define CR_WR_RDN 0x0002
+#define CPHY_TX_INPUT_STS 0x2001
#define CPHY_RX_INPUT_STS 0x2002
-#define CPHY_SATA_OVERRIDE 0x4000
-#define CPHY_OVERRIDE 0x2005
+#define CPHY_SATA_TX_OVERRIDE 0x8000
+#define CPHY_SATA_RX_OVERRIDE 0x4000
+#define CPHY_TX_OVERRIDE 0x2004
+#define CPHY_RX_OVERRIDE 0x2005
#define SPHY_LANE 0x100
#define SPHY_HALF_RATE 0x0001
#define CPHY_SATA_DPLL_MODE 0x0700
#define CPHY_SATA_DPLL_SHIFT 8
#define CPHY_SATA_DPLL_RESET (1 << 11)
+#define CPHY_SATA_TX_ATTEN 0x1c00
+#define CPHY_SATA_TX_ATTEN_SHIFT 10
#define CPHY_PHY_COUNT 6
#define CPHY_LANE_COUNT 4
#define CPHY_PORT_COUNT (CPHY_PHY_COUNT * CPHY_LANE_COUNT)
@@ -66,6 +71,7 @@ struct phy_lane_info {
void __iomem *phy_base;
u8 lane_mapping;
u8 phy_devs;
+ u8 tx_atten;
};
static struct phy_lane_info port_data[CPHY_PORT_COUNT];

@@ -76,7 +82,6 @@ static DEFINE_SPINLOCK(sgpio_lock);
#define SGPIO_PINS 3
#define SGPIO_PORTS 8

-/* can be cast as an ahci_host_priv for compatibility with most functions */
struct ecx_plat_data {
u32 n_ports;
unsigned sgpio_gpio[SGPIO_PINS];
@@ -259,8 +264,27 @@ static void highbank_cphy_disable_overrides(u8 sata_port)
if (unlikely(port_data[sata_port].phy_base == NULL))
return;
tmp = combo_phy_read(sata_port, CPHY_RX_INPUT_STS + lane * SPHY_LANE);
- tmp &= ~CPHY_SATA_OVERRIDE;
- combo_phy_write(sata_port, CPHY_OVERRIDE + lane * SPHY_LANE, tmp);
+ tmp &= ~CPHY_SATA_RX_OVERRIDE;
+ combo_phy_write(sata_port, CPHY_RX_OVERRIDE + lane * SPHY_LANE, tmp);
+}
+
+static void cphy_override_tx_attenuation(u8 sata_port, u32 val)
+{
+ u8 lane = port_data[sata_port].lane_mapping;
+ u32 tmp;
+
+ if (val & 0x8)
+ return;
+
+ tmp = combo_phy_read(sata_port, CPHY_TX_INPUT_STS + lane * SPHY_LANE);
+ tmp &= ~CPHY_SATA_TX_OVERRIDE;
+ combo_phy_write(sata_port, CPHY_TX_OVERRIDE + lane * SPHY_LANE, tmp);
+
+ tmp |= CPHY_SATA_TX_OVERRIDE;
+ combo_phy_write(sata_port, CPHY_TX_OVERRIDE + lane * SPHY_LANE, tmp);
+
+ tmp |= (val << CPHY_SATA_TX_ATTEN_SHIFT) & CPHY_SATA_TX_ATTEN;
+ combo_phy_write(sata_port, CPHY_TX_OVERRIDE + lane * SPHY_LANE, tmp);
}

static void cphy_override_rx_mode(u8 sata_port, u32 val)
@@ -268,21 +292,21 @@ static void cphy_override_rx_mode(u8 sata_port, u32 val)
u8 lane = port_data[sata_port].lane_mapping;
u32 tmp;
tmp = combo_phy_read(sata_port, CPHY_RX_INPUT_STS + lane * SPHY_LANE);
- tmp &= ~CPHY_SATA_OVERRIDE;
- combo_phy_write(sata_port, CPHY_OVERRIDE + lane * SPHY_LANE, tmp);
+ tmp &= ~CPHY_SATA_RX_OVERRIDE;
+ combo_phy_write(sata_port, CPHY_RX_OVERRIDE + lane * SPHY_LANE, tmp);

- tmp |= CPHY_SATA_OVERRIDE;
- combo_phy_write(sata_port, CPHY_OVERRIDE + lane * SPHY_LANE, tmp);
+ tmp |= CPHY_SATA_RX_OVERRIDE;
+ combo_phy_write(sata_port, CPHY_RX_OVERRIDE + lane * SPHY_LANE, tmp);

tmp &= ~CPHY_SATA_DPLL_MODE;
tmp |= val << CPHY_SATA_DPLL_SHIFT;
- combo_phy_write(sata_port, CPHY_OVERRIDE + lane * SPHY_LANE, tmp);
+ combo_phy_write(sata_port, CPHY_RX_OVERRIDE + lane * SPHY_LANE, tmp);

tmp |= CPHY_SATA_DPLL_RESET;
- combo_phy_write(sata_port, CPHY_OVERRIDE + lane * SPHY_LANE, tmp);
+ combo_phy_write(sata_port, CPHY_RX_OVERRIDE + lane * SPHY_LANE, tmp);

tmp &= ~CPHY_SATA_DPLL_RESET;
- combo_phy_write(sata_port, CPHY_OVERRIDE + lane * SPHY_LANE, tmp);
+ combo_phy_write(sata_port, CPHY_RX_OVERRIDE + lane * SPHY_LANE, tmp);

msleep(15);
}
@@ -299,16 +323,20 @@ static void highbank_cphy_override_lane(u8 sata_port)
lane * SPHY_LANE);
} while ((tmp & SPHY_HALF_RATE) && (k++ < 1000));
cphy_override_rx_mode(sata_port, 3);
+ cphy_override_tx_attenuation(sata_port, port_data[sata_port].tx_atten);
}

static int highbank_initialize_phys(struct device *dev, void __iomem *addr)
{
struct device_node *sata_node = dev->of_node;
- int phy_count = 0, phy, port = 0;
+ int phy_count = 0, phy, port = 0, i;
void __iomem *cphy_base[CPHY_PHY_COUNT];
struct device_node *phy_nodes[CPHY_PHY_COUNT];
+ u32 tx_atten[CPHY_PORT_COUNT];
+
memset(port_data, 0, sizeof(struct phy_lane_info) * CPHY_PORT_COUNT);
memset(phy_nodes, 0, sizeof(struct device_node*) * CPHY_PHY_COUNT);
+ memset(tx_atten, 0xff, CPHY_PORT_COUNT);

do {
u32 tmp;
@@ -336,6 +364,10 @@ static int highbank_initialize_phys(struct device *dev, void __iomem *addr)
of_node_put(phy_data.np);
port += 1;
} while (port < CPHY_PORT_COUNT);
+ of_property_read_u32_array(sata_node, "calxeda,tx-atten",
+ tx_atten, port);
+ for (i = 0; i < port; i++)
+ port_data[i].tx_atten = (u8) tx_atten[i];
return 0;
}

--
1.8.1.2

2013-08-07 15:55:18

by Mark Langsdorf

[permalink] [raw]
Subject: [PATCH v3 3/5] devicetree: create a separate binding description for sata_highbank

The Calxeda sata_highbank driver has been adding its descriptions to the
ahci driver. Separate them properly.

Signed-off-by: Mark Langsdorf <[email protected]>
Acked-by: Rob Herring <[email protected]>
---
Changes from v2
Fixed some indenting.
Changes from v1
None.

.../devicetree/bindings/ata/ahci-platform.txt | 18 +++---------
.../devicetree/bindings/ata/sata_highbank.txt | 32 ++++++++++++++++++++++
2 files changed, 36 insertions(+), 14 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ata/sata_highbank.txt

diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
index 3ec0c5c..89de156 100644
--- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
+++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
@@ -4,27 +4,17 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
Each SATA controller should have its own node.

Required properties:
-- compatible : compatible list, contains "calxeda,hb-ahci" or "snps,spear-ahci"
+- compatible : compatible list, contains "snps,spear-ahci"
- interrupts : <interrupt mapping for SATA IRQ>
- reg : <registers mapping>

Optional properties:
-- calxeda,port-phys: phandle-combophy and lane assignment, which maps each
- SATA port to a combophy and a lane within that
- combophy
-- calxeda,sgpio-gpio: phandle-gpio bank, bit offset, and default on or off,
- which indicates that the driver supports SGPIO
- indicator lights using the indicated GPIOs
-- calxeda,led-order : a u32 array that map port numbers to offsets within the
- SGPIO bitstream.
- dma-coherent : Present if dma operations are coherent

Example:
sata@ffe08000 {
- compatible = "calxeda,hb-ahci";
- reg = <0xffe08000 0x1000>;
- interrupts = <115>;
- calxeda,port-phys = <&combophy5 0 &combophy0 0 &combophy0 1
- &combophy0 2 &combophy0 3>;
+ compatible = "snps,spear-ahci";
+ reg = <0xffe08000 0x1000>;
+ interrupts = <115>;

};
diff --git a/Documentation/devicetree/bindings/ata/sata_highbank.txt b/Documentation/devicetree/bindings/ata/sata_highbank.txt
new file mode 100644
index 0000000..1ac6d3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/ata/sata_highbank.txt
@@ -0,0 +1,32 @@
+* Calxeda AHCI SATA Controller
+
+SATA nodes are defined to describe on-chip Serial ATA controllers.
+The Calxeda SATA controller mostly conforms to the AHCI interface
+with some special extensions to add functionality.
+Each SATA controller should have its own node.
+
+Required properties:
+- compatible : compatible list, contains "calxeda,hb-ahci"
+- interrupts : <interrupt mapping for SATA IRQ>
+- reg : <registers mapping>
+
+Optional properties:
+- dma-coherent : Present if dma operations are coherent
+- calxeda,port-phys: phandle-combophy and lane assignment, which maps each
+ SATA port to a combophy and a lane within that
+ combophy
+- calxeda,sgpio-gpio: phandle-gpio bank, bit offset, and default on or off,
+ which indicates that the driver supports SGPIO
+ indicator lights using the indicated GPIOs
+- calxeda,led-order : a u32 array that map port numbers to offsets within the
+ SGPIO bitstream.
+
+Example:
+ sata@ffe08000 {
+ compatible = "calxeda,hb-ahci";
+ reg = <0xffe08000 0x1000>;
+ interrupts = <115>;
+ calxeda,port-phys = <&combophy5 0 &combophy0 0 &combophy0 1
+ &combophy0 2 &combophy0 3>;
+
+ };
--
1.8.1.2

2013-08-07 16:12:59

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] devicetree: create a separate binding description for sata_highbank


On Aug 7, 2013, at 10:52 AM, Mark Langsdorf wrote:

> The Calxeda sata_highbank driver has been adding its descriptions to the
> ahci driver. Separate them properly.
>
> Signed-off-by: Mark Langsdorf <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> ---
> Changes from v2
> Fixed some indenting.
> Changes from v1
> None.
>
> .../devicetree/bindings/ata/ahci-platform.txt | 18 +++---------
> .../devicetree/bindings/ata/sata_highbank.txt | 32 ++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 14 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/ata/sata_highbank.txt
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index 3ec0c5c..89de156 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -4,27 +4,17 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
> Each SATA controller should have its own node.
>
> Required properties:
> -- compatible : compatible list, contains "calxeda,hb-ahci" or "snps,spear-ahci"
> +- compatible : compatible list, contains "snps,spear-ahci"
> - interrupts : <interrupt mapping for SATA IRQ>
> - reg : <registers mapping>
>
> Optional properties:
> -- calxeda,port-phys: phandle-combophy and lane assignment, which maps each
> - SATA port to a combophy and a lane within that
> - combophy
> -- calxeda,sgpio-gpio: phandle-gpio bank, bit offset, and default on or off,
> - which indicates that the driver supports SGPIO
> - indicator lights using the indicated GPIOs
> -- calxeda,led-order : a u32 array that map port numbers to offsets within the
> - SGPIO bitstream.
> - dma-coherent : Present if dma operations are coherent
>
> Example:
> sata@ffe08000 {
> - compatible = "calxeda,hb-ahci";
> - reg = <0xffe08000 0x1000>;
> - interrupts = <115>;
> - calxeda,port-phys = <&combophy5 0 &combophy0 0 &combophy0 1
> - &combophy0 2 &combophy0 3>;
> + compatible = "snps,spear-ahci";
> + reg = <0xffe08000 0x1000>;
> + interrupts = <115>;
>
> };
> diff --git a/Documentation/devicetree/bindings/ata/sata_highbank.txt b/Documentation/devicetree/bindings/ata/sata_highbank.txt
> new file mode 100644
> index 0000000..1ac6d3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/sata_highbank.txt
> @@ -0,0 +1,32 @@
> +* Calxeda AHCI SATA Controller
> +
> +SATA nodes are defined to describe on-chip Serial ATA controllers.
> +The Calxeda SATA controller mostly conforms to the AHCI interface
> +with some special extensions to add functionality.
> +Each SATA controller should have its own node.
> +
> +Required properties:
> +- compatible : compatible list, contains "calxeda,hb-ahci"
> +- interrupts : <interrupt mapping for SATA IRQ>
> +- reg : <registers mapping>
> +
> +Optional properties:
> +- dma-coherent : Present if dma operations are coherent
> +- calxeda,port-phys: phandle-combophy and lane assignment, which maps each
> + SATA port to a combophy and a lane within that
> + combophy
> +- calxeda,sgpio-gpio: phandle-gpio bank, bit offset, and default on or off,
> + which indicates that the driver supports SGPIO
> + indicator lights using the indicated GPIOs
> +- calxeda,led-order : a u32 array that map port numbers to offsets within the
> + SGPIO bitstream.

nit: whitespace after :

> +
> +Example:
> + sata@ffe08000 {
> + compatible = "calxeda,hb-ahci";
> + reg = <0xffe08000 0x1000>;
> + interrupts = <115>;
> + calxeda,port-phys = <&combophy5 0 &combophy0 0 &combophy0 1
> + &combophy0 2 &combophy0 3>;
> +

Its probably good to show all optional props (dma-coherent, calxeda,sgpio-gpios, & calxeda,led-order) in the example.

> + };
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2013-08-08 09:46:18

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] devicetree: create a separate binding description for sata_highbank

On Wed, Aug 07, 2013 at 04:52:36PM +0100, Mark Langsdorf wrote:
> The Calxeda sata_highbank driver has been adding its descriptions to the
> ahci driver. Separate them properly.
>
> Signed-off-by: Mark Langsdorf <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> ---
> Changes from v2
> Fixed some indenting.
> Changes from v1
> None.
>
> .../devicetree/bindings/ata/ahci-platform.txt | 18 +++---------
> .../devicetree/bindings/ata/sata_highbank.txt | 32 ++++++++++++++++++++++
> 2 files changed, 36 insertions(+), 14 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/ata/sata_highbank.txt
>
> diff --git a/Documentation/devicetree/bindings/ata/ahci-platform.txt b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> index 3ec0c5c..89de156 100644
> --- a/Documentation/devicetree/bindings/ata/ahci-platform.txt
> +++ b/Documentation/devicetree/bindings/ata/ahci-platform.txt
> @@ -4,27 +4,17 @@ SATA nodes are defined to describe on-chip Serial ATA controllers.
> Each SATA controller should have its own node.
>
> Required properties:
> -- compatible : compatible list, contains "calxeda,hb-ahci" or "snps,spear-ahci"
> +- compatible : compatible list, contains "snps,spear-ahci"
> - interrupts : <interrupt mapping for SATA IRQ>
> - reg : <registers mapping>
>
> Optional properties:
> -- calxeda,port-phys: phandle-combophy and lane assignment, which maps each
> - SATA port to a combophy and a lane within that
> - combophy
> -- calxeda,sgpio-gpio: phandle-gpio bank, bit offset, and default on or off,
> - which indicates that the driver supports SGPIO
> - indicator lights using the indicated GPIOs
> -- calxeda,led-order : a u32 array that map port numbers to offsets within the
> - SGPIO bitstream.
> - dma-coherent : Present if dma operations are coherent
>
> Example:
> sata@ffe08000 {
> - compatible = "calxeda,hb-ahci";
> - reg = <0xffe08000 0x1000>;
> - interrupts = <115>;
> - calxeda,port-phys = <&combophy5 0 &combophy0 0 &combophy0 1
> - &combophy0 2 &combophy0 3>;
> + compatible = "snps,spear-ahci";
> + reg = <0xffe08000 0x1000>;
> + interrupts = <115>;
>
> };
> diff --git a/Documentation/devicetree/bindings/ata/sata_highbank.txt b/Documentation/devicetree/bindings/ata/sata_highbank.txt
> new file mode 100644
> index 0000000..1ac6d3d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/sata_highbank.txt
> @@ -0,0 +1,32 @@
> +* Calxeda AHCI SATA Controller
> +
> +SATA nodes are defined to describe on-chip Serial ATA controllers.
> +The Calxeda SATA controller mostly conforms to the AHCI interface
> +with some special extensions to add functionality.
> +Each SATA controller should have its own node.
> +
> +Required properties:
> +- compatible : compatible list, contains "calxeda,hb-ahci"
> +- interrupts : <interrupt mapping for SATA IRQ>
> +- reg : <registers mapping>
> +
> +Optional properties:
> +- dma-coherent : Present if dma operations are coherent
> +- calxeda,port-phys: phandle-combophy and lane assignment, which maps each
> + SATA port to a combophy and a lane within that
> + combophy
> +- calxeda,sgpio-gpio: phandle-gpio bank, bit offset, and default on or off,
> + which indicates that the driver supports SGPIO
> + indicator lights using the indicated GPIOs
> +- calxeda,led-order : a u32 array that map port numbers to offsets within the
> + SGPIO bitstream.
> +
> +Example:
> + sata@ffe08000 {
> + compatible = "calxeda,hb-ahci";
> + reg = <0xffe08000 0x1000>;
> + interrupts = <115>;
> + calxeda,port-phys = <&combophy5 0 &combophy0 0 &combophy0 1
> + &combophy0 2 &combophy0 3>;
> +
> + };

It would be nice to have examples for the remaining optional properties,
but otherwise:

Acked-by: Mark Rutland <[email protected]>

2013-08-09 16:08:05

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] sata, highbank: fix ordering of SGPIO signals

On Wed, Aug 07, 2013 at 10:52:34AM -0500, Mark Langsdorf wrote:
> The ACTIVITY and ERROR signals were reversed in the original commit.
> Fix that so that hard drive activity does not show up on the error
> light, and attempts to indicate that the hard drive is failing do
> not show up as hard drive activity. This fixes a fairly serious
> functional bug in the driver, but failing to apply this patch will
> not cause any stability issues on the system.
>
> Signed-off-by: Mark Langsdorf <[email protected]>

Applied to libata/for-3.11-fixes w/ stable cc'd.

Thanks.

--
tejun

2013-08-09 16:08:58

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] sata highbank: enable 64-bit DMA mask when using LPAE

On Wed, Aug 07, 2013 at 10:52:35AM -0500, Mark Langsdorf wrote:
> From: Rob Herring <[email protected]>
>
> Signed-off-by: Rob Herring <[email protected]>
> Signed-off-by: Mark Langsdorf <[email protected]>

Applied to libata/for-3.12.

Thanks.

--
tejun

2013-08-09 16:10:23

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] devicetree: create a separate binding description for sata_highbank

On Wed, Aug 07, 2013 at 10:52:36AM -0500, Mark Langsdorf wrote:
> The Calxeda sata_highbank driver has been adding its descriptions to the
> ahci driver. Separate them properly.
>
> Signed-off-by: Mark Langsdorf <[email protected]>
> Acked-by: Rob Herring <[email protected]>

Will wait for the update. I suppose this doesn't need to go in to
for-3.11-fixes? I'd really appreciate what impact each patch has.

Thanks.

--
tejun

2013-08-09 16:10:50

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] devicetree: create a separate binding description for sata_highbank

On Fri, Aug 09, 2013 at 12:10:15PM -0400, Tejun Heo wrote:
> Will wait for the update. I suppose this doesn't need to go in to
> for-3.11-fixes? I'd really appreciate what impact each patch has.
^
descriptions of
--
tejun

2013-08-09 16:12:40

by Mark Langsdorf

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] devicetree: create a separate binding description for sata_highbank

On 08/09/2013 11:10 AM, Tejun Heo wrote:
> On Wed, Aug 07, 2013 at 10:52:36AM -0500, Mark Langsdorf wrote:
>> The Calxeda sata_highbank driver has been adding its descriptions to the
>> ahci driver. Separate them properly.
>>
>> Signed-off-by: Mark Langsdorf <[email protected]>
>> Acked-by: Rob Herring <[email protected]>
>
> Will wait for the update. I suppose this doesn't need to go in to
> for-3.11-fixes? I'd really appreciate what impact each patch has.

I'll update the description when I resubmit it to give you more detail
on the impact.

Thanks for applying the others.

--Mark Langsdorf
Calxeda, Inc.

2013-08-09 16:15:50

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] devicetree: create a separate binding description for sata_highbank

Hello, Mark.

On Fri, Aug 9, 2013 at 12:12 PM, Mark Langsdorf
<[email protected]> wrote:
> I'll update the description when I resubmit it to give you more detail on
> the impact.

Yes, please give me some hints on to which branch the patches are
targeted and whether stable should be cc'd. I can't really tell just
looking at the patches as I have no idea how widespread the device is
in the wild and which kernels they'd be running or how critical being
out-of-spec is (is it gonna burn phys on the receiving end in the long
term? or is it just noisier and the link is gonna be more susceptible
to errors from EM interferences and so on) and how risky the fixes
are.

Thanks.

--
tejun