On 01/28, Andy Gross wrote:

On Wed, Jan 28, 2015 at 06:11:50PM -0800, Stephen Boyd wrote:

quoted

quoted

+- syscon-tcsr: indicates phandle of TCSR syscon node

Make this optional but required if any child nodes use dma?

To enforce that I'd have to determine that a child has a dmas.  I guess that
isn't so bad.

I don't think we need to write any code in the driver to enforce
this right now. As long as nobody gets it wrong bad things won't
happen and we can just assume that the DT is correct. Once we get
some bad DT we can go ahead and implement some DT parsing stuff.

quoted

quoted

+
+	if (!gsbi_num || gsbi_num > MAX_GSBI) {
+		dev_err(&pdev->dev, "invalid gsbi number\n");
+		return -EINVAL;
+	}
+
 	if (of_property_read_u32(node, "qcom,mode", &gsbi->mode)) {
 		dev_err(&pdev->dev, "missing mode configuration\n");
 		return -EINVAL;

@@ -64,6 +185,26 @@ static int gsbi_probe(struct platform_device *pdev)
 	writel_relaxed((gsbi->mode << GSBI_PROTOCOL_SHIFT) | gsbi->crci,
 				base + GSBI_CTRL_REG);
 
+	/*
+	 * modify tcsr to reflect mode and ADM CRCI mux
+	 * Each gsbi contains a pair of bits, one for RX and one for TX
+	 * SPI mode requires both bits cleared, otherwise they are set
+	 */
+	match = of_match_node(gsbi_dt_match, node);

Why not match the config to the TCSR compatible string? Wouldn't
that more accurately reflect that we need to set different bits
depending on which type of TCSR we're using? The version of GSBI
hardware is not actually changing in every different SoC so I
don't see why we want to change the compatible there just because
the TCSR register layout changed.

That is true.  However, with the gsbi compat, I avoid doing a match multiple
times and get the table I need immediately.  The alternative is N checks or
pulling the compat strings and comparing them to get the right table.

Sorry I'm not following. We always have to call of_match_node()
here and I'm just suggesting we replace gsbi_dt_match with
tcsr_dt_match and leave gsbi_dt_match like it was before. I don't
see how anything is avoided.

quoted

quoted

+
+	if (config)
+		for (i = 0; i < config->num_rows; i++) {
+			if (gsbi->mode == GSBI_PROT_SPI)

Doesn't I2C need the same treatment (anything in QUP really)?
Maybe the logic could be changed to check for gsbi->crci ==
GSBI_CRCI_QUP?

Nope.  I2C doesn't support DMA when ADM is the controller.  It's only SPI or
UART.

Ok, I don't get it but I guess it doesn't matter if I2C doesn't
use it.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project