* [PATCH 06/14] ASoC: Add sun8i digital audio codec
From: Mark Brown @ 2016-10-04 13:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004144008.0d07d18c@free-electrons.com>
On Tue, Oct 04, 2016 at 02:40:08PM +0200, Thomas Petazzoni wrote:
> > +/* CODEC_OFFSET represents the offset of the codec registers
> > + * and not all the DAI registers
> > + */
> This is not the proper comment style I believe for audio code, it
> should be:
> /*
> * ...
> */
I don't care, IIRC that's something from CodingStyle which checkpatch
moans about.
> > + /* pcm operations */
> > + .ops = &sun8i_codec_dai_ops,
> > +};
> > +EXPORT_SYMBOL(sun8i_codec_dai);
> This EXPORT_SYMBOL looks wrong. First because it doesn't seem to be
> used outside of this module. And second because using EXPORT_SYMBOL on
> a function defined as static doesn't make much sense, as the "static"
> qualifier limits the visibility of the symbol to the current
> compilation unit.
Also all the ASoC code is _GPL so a non-GPL export is an issue.
> > + .component_driver = {
> > + .dapm_widgets = sun8i_codec_dapm_widgets,
> > + .num_dapm_widgets = ARRAY_SIZE(sun8i_codec_dapm_widgets),
> > + .dapm_routes = sun8i_codec_dapm_routes,
> > + .num_dapm_routes = ARRAY_SIZE(sun8i_codec_dapm_routes),
> I'm probably missing something, but in the sun4i-codec.c driver, those
> fields are initialized directly in the snd_soc_codec_driver structure,
> not in the .component_driver sub-structure.
We're in the process of pushing everything out to component level, this
update should be made in the old code if it's not happened already.
> > + if (clk_prepare_enable(scodec->clk_module))
> > + pr_err("err:open failed;\n");
> Grr, pr_err, not good. Plus you want to return with an error from the
> probe() function.
Also when printing an error message use dev_err().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161004/4287ed0a/attachment.sig>
^ permalink raw reply
* [PATCH 6/6] ARM: da850: adjust memory settings for tilcdc
From: Kevin Hilman @ 2016-10-04 13:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0125defa-5c34-85d0-f62a-133235c5094c@ti.com>
Peter Ujfalusi <peter.ujfalusi@ti.com> writes:
> On 10/01/16 12:24, Sekhar Nori wrote:
[...]
>> In any case, to configure the PBBR, you will have to introduce a driver
>> for it in drivers/memory. Then you can set it up per board using a DT
>> parameter.
>
> and we can reuse the introduced bindings for am335x and OMAP1/2 as well. On
> OMAP the legacy DMA API provided a call to raise the priority of the sDMA in
> EMIF :o That needs to be removed and replaced.
Can you point us to the bindings you're referring to?
Also, a new driver in drivers/memory is fine for setting the PBBR, but
what about the SYSCFG0 registers. Are you OK with leaving those in the
init code as proposed in $SUBJECT patch?
Kevin
^ permalink raw reply
* [PATCH v3 2/2] crypto: marvell - Don't break chain for computable last ahash requests
From: Romain Perier @ 2016-10-04 12:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004125720.3347-1-romain.perier@free-electrons.com>
Currently, the driver breaks chain for all kind of hash requests in order to
don't override intermediate states of partial ahash updates. However, some final
ahash requests can be directly processed by the engine, and so without
intermediate state. This is typically the case for most for the HMAC requests
processed via IPSec.
This commits adds a TDMA descriptor to copy context for these of requests
into the "op" dma pool, then it allow to chain these requests at the DMA level.
The 'complete' operation is also updated to retrieve the MAC digest from the
right location.
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
Changes in v3:
- Copy the whole context back to RAM and not just the digest. Also
fixed a rebase issue ^^ (whoops)
Changes in v2:
- Replaced BUG_ON by an error
- Add a variable "break_chain", with "type" to break the chain
with ahash requests. It improves code readability.
drivers/crypto/marvell/hash.c | 79 +++++++++++++++++++++++++++++++++++--------
1 file changed, 64 insertions(+), 15 deletions(-)
diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 9f28468..b36f196 100644
--- a/drivers/crypto/marvell/hash.c
+++ b/drivers/crypto/marvell/hash.c
@@ -312,24 +312,53 @@ static void mv_cesa_ahash_complete(struct crypto_async_request *req)
int i;
digsize = crypto_ahash_digestsize(crypto_ahash_reqtfm(ahashreq));
- for (i = 0; i < digsize / 4; i++)
- creq->state[i] = readl_relaxed(engine->regs + CESA_IVDIG(i));
- if (creq->last_req) {
+ if (mv_cesa_req_get_type(&creq->base) == CESA_DMA_REQ &&
+ !(creq->base.chain.last->flags & CESA_TDMA_BREAK_CHAIN)) {
+ struct mv_cesa_tdma_desc *tdma = NULL;
+ __le32 *data = NULL;
+
+ for (tdma = creq->base.chain.first; tdma; tdma = tdma->next) {
+ u32 type = tdma->flags & CESA_TDMA_TYPE_MSK;
+ if (type == CESA_TDMA_RESULT)
+ break;
+ }
+
+ if (!tdma) {
+ dev_err(cesa_dev->dev, "Failed to retrieve tdma "
+ "descriptor for outer data\n");
+ return;
+ }
+
/*
- * Hardware's MD5 digest is in little endian format, but
- * SHA in big endian format
+ * Result is already in the correct endianess when the SA is
+ * used
*/
- if (creq->algo_le) {
- __le32 *result = (void *)ahashreq->result;
+ data = tdma->op->ctx.hash.hash;
+ for (i = 0; i < digsize / 4; i++)
+ creq->state[i] = cpu_to_le32(data[i]);
- for (i = 0; i < digsize / 4; i++)
- result[i] = cpu_to_le32(creq->state[i]);
- } else {
- __be32 *result = (void *)ahashreq->result;
+ memcpy(ahashreq->result, data, digsize);
+ } else {
+ for (i = 0; i < digsize / 4; i++)
+ creq->state[i] = readl_relaxed(engine->regs +
+ CESA_IVDIG(i));
+ if (creq->last_req) {
+ /*
+ * Hardware's MD5 digest is in little endian format, but
+ * SHA in big endian format
+ */
+ if (creq->algo_le) {
+ __le32 *result = (void *)ahashreq->result;
+
+ for (i = 0; i < digsize / 4; i++)
+ result[i] = cpu_to_le32(creq->state[i]);
+ } else {
+ __be32 *result = (void *)ahashreq->result;
- for (i = 0; i < digsize / 4; i++)
- result[i] = cpu_to_be32(creq->state[i]);
+ for (i = 0; i < digsize / 4; i++)
+ result[i] = cpu_to_be32(creq->state[i]);
+ }
}
}
@@ -504,6 +533,12 @@ mv_cesa_ahash_dma_last_req(struct mv_cesa_tdma_chain *chain,
CESA_SA_DESC_CFG_LAST_FRAG,
CESA_SA_DESC_CFG_FRAG_MSK);
+ ret = mv_cesa_dma_add_result_op(chain,
+ CESA_SA_CFG_SRAM_OFFSET,
+ CESA_SA_DATA_SRAM_OFFSET,
+ CESA_TDMA_SRC_IN_SRAM, flags);
+ if (ret)
+ return ERR_PTR(-ENOMEM);
return op;
}
@@ -564,6 +599,8 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
struct mv_cesa_op_ctx *op = NULL;
unsigned int frag_len;
int ret;
+ u32 type;
+ bool break_chain = true;
basereq->chain.first = NULL;
basereq->chain.last = NULL;
@@ -635,6 +672,16 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
goto err_free_tdma;
}
+ /*
+ * If results are copied via DMA, this means that this
+ * request can be directly processed by the engine,
+ * without partial updates. So we can chain it at the
+ * DMA level with other requests.
+ */
+ type = basereq->chain.last->flags & CESA_TDMA_TYPE_MSK;
+ if (type == CESA_TDMA_RESULT)
+ break_chain = false;
+
if (op) {
/* Add dummy desc to wait for crypto operation end */
ret = mv_cesa_dma_add_dummy_end(&basereq->chain, flags);
@@ -648,8 +695,10 @@ static int mv_cesa_ahash_dma_req_init(struct ahash_request *req)
else
creq->cache_ptr = 0;
- basereq->chain.last->flags |= (CESA_TDMA_END_OF_REQ |
- CESA_TDMA_BREAK_CHAIN);
+ basereq->chain.last->flags |= CESA_TDMA_END_OF_REQ;
+
+ if (break_chain)
+ basereq->chain.last->flags |= CESA_TDMA_BREAK_CHAIN;
return 0;
--
2.9.3
^ permalink raw reply related
* [PATCH v3 1/2] crypto: marvell - Use an unique pool to copy results of requests
From: Romain Perier @ 2016-10-04 12:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004125720.3347-1-romain.perier@free-electrons.com>
So far, we used a dedicated dma pool to copy the result of outer IV for
cipher requests. Instead of using a dma pool per outer data, we prefer
use the op dma pool that contains all part of the request from the SRAM.
Then, the outer data that is likely to be used by the 'complete'
operation, is copied later. In this way, any type of result can be
retrieved by DMA for cipher or ahash requests.
Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
Changes in v3:
- Don't allocate a new op ctx for the last tdma descriptor. Instead
we point to the last op ctx in the tdma chain, and copy the context
of the current request to this location.
Changes in v2:
- Use the dma pool "op" to retrieve outer data intead of introducing
a new one.
drivers/crypto/marvell/cesa.c | 4 ----
drivers/crypto/marvell/cesa.h | 5 ++---
drivers/crypto/marvell/cipher.c | 8 +++++---
drivers/crypto/marvell/tdma.c | 28 ++++++++++++++--------------
4 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
index 37dadb2..6e7a5c7 100644
--- a/drivers/crypto/marvell/cesa.c
+++ b/drivers/crypto/marvell/cesa.c
@@ -375,10 +375,6 @@ static int mv_cesa_dev_dma_init(struct mv_cesa_dev *cesa)
if (!dma->padding_pool)
return -ENOMEM;
- dma->iv_pool = dmam_pool_create("cesa_iv", dev, 16, 1, 0);
- if (!dma->iv_pool)
- return -ENOMEM;
-
cesa->dma = dma;
return 0;
diff --git a/drivers/crypto/marvell/cesa.h b/drivers/crypto/marvell/cesa.h
index e423d33..a768da7 100644
--- a/drivers/crypto/marvell/cesa.h
+++ b/drivers/crypto/marvell/cesa.h
@@ -277,7 +277,7 @@ struct mv_cesa_op_ctx {
#define CESA_TDMA_DUMMY 0
#define CESA_TDMA_DATA 1
#define CESA_TDMA_OP 2
-#define CESA_TDMA_IV 3
+#define CESA_TDMA_RESULT 3
/**
* struct mv_cesa_tdma_desc - TDMA descriptor
@@ -393,7 +393,6 @@ struct mv_cesa_dev_dma {
struct dma_pool *op_pool;
struct dma_pool *cache_pool;
struct dma_pool *padding_pool;
- struct dma_pool *iv_pool;
};
/**
@@ -839,7 +838,7 @@ mv_cesa_tdma_desc_iter_init(struct mv_cesa_tdma_chain *chain)
memset(chain, 0, sizeof(*chain));
}
-int mv_cesa_dma_add_iv_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
+int mv_cesa_dma_add_result_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
u32 size, u32 flags, gfp_t gfp_flags);
struct mv_cesa_op_ctx *mv_cesa_dma_add_op(struct mv_cesa_tdma_chain *chain,
diff --git a/drivers/crypto/marvell/cipher.c b/drivers/crypto/marvell/cipher.c
index d19dc96..098871a 100644
--- a/drivers/crypto/marvell/cipher.c
+++ b/drivers/crypto/marvell/cipher.c
@@ -212,7 +212,8 @@ mv_cesa_ablkcipher_complete(struct crypto_async_request *req)
struct mv_cesa_req *basereq;
basereq = &creq->base;
- memcpy(ablkreq->info, basereq->chain.last->data, ivsize);
+ memcpy(ablkreq->info, basereq->chain.last->op->ctx.blkcipher.iv,
+ ivsize);
} else {
memcpy_fromio(ablkreq->info,
engine->sram + CESA_SA_CRYPT_IV_SRAM_OFFSET,
@@ -373,8 +374,9 @@ static int mv_cesa_ablkcipher_dma_req_init(struct ablkcipher_request *req,
/* Add output data for IV */
ivsize = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
- ret = mv_cesa_dma_add_iv_op(&basereq->chain, CESA_SA_CRYPT_IV_SRAM_OFFSET,
- ivsize, CESA_TDMA_SRC_IN_SRAM, flags);
+ ret = mv_cesa_dma_add_result_op(&basereq->chain, CESA_SA_CFG_SRAM_OFFSET,
+ CESA_SA_DATA_SRAM_OFFSET,
+ CESA_TDMA_SRC_IN_SRAM, flags);
if (ret)
goto err_free_tdma;
diff --git a/drivers/crypto/marvell/tdma.c b/drivers/crypto/marvell/tdma.c
index 9fd7a5f..991dc3f 100644
--- a/drivers/crypto/marvell/tdma.c
+++ b/drivers/crypto/marvell/tdma.c
@@ -69,9 +69,6 @@ void mv_cesa_dma_cleanup(struct mv_cesa_req *dreq)
if (type == CESA_TDMA_OP)
dma_pool_free(cesa_dev->dma->op_pool, tdma->op,
le32_to_cpu(tdma->src));
- else if (type == CESA_TDMA_IV)
- dma_pool_free(cesa_dev->dma->iv_pool, tdma->data,
- le32_to_cpu(tdma->dst));
tdma = tdma->next;
dma_pool_free(cesa_dev->dma->tdma_desc_pool, old_tdma,
@@ -209,29 +206,32 @@ mv_cesa_dma_add_desc(struct mv_cesa_tdma_chain *chain, gfp_t flags)
return new_tdma;
}
-int mv_cesa_dma_add_iv_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
+int mv_cesa_dma_add_result_op(struct mv_cesa_tdma_chain *chain, dma_addr_t src,
u32 size, u32 flags, gfp_t gfp_flags)
{
-
- struct mv_cesa_tdma_desc *tdma;
- u8 *iv;
- dma_addr_t dma_handle;
+ struct mv_cesa_tdma_desc *tdma, *op_desc;
tdma = mv_cesa_dma_add_desc(chain, gfp_flags);
if (IS_ERR(tdma))
return PTR_ERR(tdma);
- iv = dma_pool_alloc(cesa_dev->dma->iv_pool, gfp_flags, &dma_handle);
- if (!iv)
- return -ENOMEM;
+ for (op_desc = chain->first; op_desc; op_desc = op_desc->next) {
+ u32 type = op_desc->flags & CESA_TDMA_TYPE_MSK;
+
+ if (type == CESA_TDMA_OP)
+ break;
+ }
+
+ if (!op_desc)
+ return -EIO;
tdma->byte_cnt = cpu_to_le32(size | BIT(31));
tdma->src = src;
- tdma->dst = cpu_to_le32(dma_handle);
- tdma->data = iv;
+ tdma->dst = op_desc->src;
+ tdma->op = op_desc->op;
flags &= (CESA_TDMA_DST_IN_SRAM | CESA_TDMA_SRC_IN_SRAM);
- tdma->flags = flags | CESA_TDMA_IV;
+ tdma->flags = flags | CESA_TDMA_RESULT;
return 0;
}
--
2.9.3
^ permalink raw reply related
* [PATCH v3 0/2] Improve DMA chaining for ahash requests
From: Romain Perier @ 2016-10-04 12:57 UTC (permalink / raw)
To: linux-arm-kernel
This series contain performance improvement regarding ahash requests.
So far, ahash requests were systematically not chained at the DMA level.
However, in some case, like this is the case by using IPSec, some ahash
requests can be processed directly by the engine, and don't have
intermediaire partial update states.
This series firstly re-work the way outer IVs are copied from the SRAM
into the dma pool. To do so, we introduce a common dma pool for all type
of requests that contains outer results (like IV or digest). Then, for
ahash requests that can be processed directly by the engine, outer
results are copied from the SRAM into the common dma pool. These requests
are then allowed to be chained at the DMA level.
Benchmarking results with iperf throught IPSec
==============================================
ESP AH
Before 343 Mbits/s 492 Mbits/s
After 422 Mbits/s 577 Mbits/s
Improvement +23% +17%
Romain Perier (2):
crypto: marvell - Use an unique pool to copy results of requests
crypto: marvell - Don't break chain for computable last ahash requests
drivers/crypto/marvell/cesa.c | 4 ---
drivers/crypto/marvell/cesa.h | 5 ++-
drivers/crypto/marvell/cipher.c | 8 +++--
drivers/crypto/marvell/hash.c | 79 +++++++++++++++++++++++++++++++++--------
drivers/crypto/marvell/tdma.c | 28 +++++++--------
5 files changed, 85 insertions(+), 39 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH v2 4/4] ARM: dts: dra72-evm-revc: fix correct phy delay
From: Mugunthan V N @ 2016-10-04 12:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004125607.10569-1-mugunthanvnm@ti.com>
The current delay settings of the phy are not the optimal value,
fix it with correct values.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
arch/arm/boot/dts/dra72-evm-revc.dts | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
index d626cd7..8472a8c 100644
--- a/arch/arm/boot/dts/dra72-evm-revc.dts
+++ b/arch/arm/boot/dts/dra72-evm-revc.dts
@@ -59,16 +59,16 @@
&davinci_mdio {
dp83867_0: ethernet-phy at 2 {
reg = <2>;
- ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
- ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
+ ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
+ ti,tx-internal-delay = <DP83867_RGMIIDCTL_250_PS>;
ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
ti,min-output-imepdance;
};
dp83867_1: ethernet-phy at 3 {
reg = <3>;
- ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
- ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
+ ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
+ ti,tx-internal-delay = <DP83867_RGMIIDCTL_250_PS>;
ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
ti,min-output-imepdance;
};
--
2.10.0.372.g6fe1b14
^ permalink raw reply related
* [PATCH v2 3/4] ARM: dts: dra72-evm-revc: fix correct phy delay and impedance settings
From: Mugunthan V N @ 2016-10-04 12:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004125607.10569-1-mugunthanvnm@ti.com>
The default impedance settings of the phy is not the optimal
value, due to this the second ethernet is not working. Fix it
with correct values which makes the second ethernet port to work.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
arch/arm/boot/dts/dra72-evm-revc.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
index f9cfd3b..d626cd7 100644
--- a/arch/arm/boot/dts/dra72-evm-revc.dts
+++ b/arch/arm/boot/dts/dra72-evm-revc.dts
@@ -62,6 +62,7 @@
ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
+ ti,min-output-imepdance;
};
dp83867_1: ethernet-phy at 3 {
@@ -69,5 +70,6 @@
ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
+ ti,min-output-imepdance;
};
};
--
2.10.0.372.g6fe1b14
^ permalink raw reply related
* [PATCH v2 2/4] net: phy: dp83867: add support for MAC impedance configuration
From: Mugunthan V N @ 2016-10-04 12:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004125607.10569-1-mugunthanvnm@ti.com>
Add support for programmable MAC impedance configuration
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
drivers/net/phy/dp83867.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 91177a4..795ae17 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -33,6 +33,7 @@
/* Extended Registers */
#define DP83867_RGMIICTL 0x0032
#define DP83867_RGMIIDCTL 0x0086
+#define DP83867_IO_MUX_CFG 0x0170
#define DP83867_SW_RESET BIT(15)
#define DP83867_SW_RESTART BIT(14)
@@ -62,10 +63,17 @@
/* RGMIIDCTL bits */
#define DP83867_RGMII_TX_CLK_DELAY_SHIFT 4
+/* IO_MUX_CFG bits */
+#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL 0x1f
+
+#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX 0x0
+#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN 0x1f
+
struct dp83867_private {
int rx_id_delay;
int tx_id_delay;
int fifo_depth;
+ int io_impedance;
};
static int dp83867_ack_interrupt(struct phy_device *phydev)
@@ -111,6 +119,14 @@ static int dp83867_of_init(struct phy_device *phydev)
if (!of_node)
return -ENODEV;
+ dp83867->io_impedance = -EINVAL;
+
+ /* Optional configuration */
+ if (of_property_read_bool(of_node, "ti,max-output-imepdance"))
+ dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
+ else if (of_property_read_bool(of_node, "ti,min-output-imepdance"))
+ dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN;
+
ret = of_property_read_u32(of_node, "ti,rx-internal-delay",
&dp83867->rx_id_delay);
if (ret)
@@ -184,6 +200,18 @@ static int dp83867_config_init(struct phy_device *phydev)
phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
DP83867_DEVADDR, delay);
+
+ if (dp83867->io_impedance >= 0) {
+ val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
+ DP83867_DEVADDR);
+
+ val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
+ val |= dp83867->io_impedance &
+ DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
+
+ phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
+ DP83867_DEVADDR, val);
+ }
}
return 0;
--
2.10.0.372.g6fe1b14
^ permalink raw reply related
* [PATCH v2 1/4] net: phy: dp83867: Add documentation for optional impedance control
From: Mugunthan V N @ 2016-10-04 12:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004125607.10569-1-mugunthanvnm@ti.com>
Add documention of ti,impedance-control which can be used to
correct MAC impedance mismatch using phy extended registers.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
Documentation/devicetree/bindings/net/ti,dp83867.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.txt b/Documentation/devicetree/bindings/net/ti,dp83867.txt
index 5d21141..85bf945 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.txt
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.txt
@@ -9,6 +9,18 @@ Required properties:
- ti,fifo-depth - Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h
for applicable values
+Optional property:
+ - ti,min-output-impedance - MAC Interface Impedance control to set
+ the programmable output impedance to
+ minimum value (35 ohms).
+ - ti,max-output-impedance - MAC Interface Impedance control to set
+ the programmable output impedance to
+ maximum value (70 ohms).
+
+Note: ti,min-output-impedance and ti,max-output-impedance are mutually
+ exclusive. When both properties are present ti,max-output-impedance
+ takes precedence.
+
Default child nodes are standard Ethernet PHY device
nodes as described in Documentation/devicetree/bindings/net/phy.txt
--
2.10.0.372.g6fe1b14
^ permalink raw reply related
* [PATCH v2 0/4] add support for impedance control for TI dp83867 phy and fix 2nd ethernet on dra72 rev C evm
From: Mugunthan V N @ 2016-10-04 12:56 UTC (permalink / raw)
To: linux-arm-kernel
Add support for configurable impedance control for TI dp83867
phy via devicetree. More documentation in [1].
CPSW second ethernet is not working, fix it by enabling
impedance configuration on the phy.
Verified the patch on DRA72 Rev C evm, logs at [2]. Also pushed
a branch [3] for others to test.
Changes from initial version:
* As per Sekhar's comment, instead of passing impedance values,
change to max and min impedance from DT
* Adopted phy_read_mmd_indirect() to cunnrent implementation.
* Corrected the phy delay timings to the optimal value.
[1] - http://www.ti.com/lit/ds/symlink/dp83867ir.pdf
[2] - http://pastebin.ubuntu.com/23274616/
[3] - git://git.ti.com/~mugunthanvnm/ti-linux-kernel/linux.git dp83867-v2
Mugunthan V N (4):
net: phy: dp83867: Add documentation for optional impedance control
net: phy: dp83867: add support for MAC impedance configuration
ARM: dts: dra72-evm-revc: fix correct phy delay and impedance settings
ARM: dts: dra72-evm-revc: fix correct phy delay
.../devicetree/bindings/net/ti,dp83867.txt | 12 ++++++++++
arch/arm/boot/dts/dra72-evm-revc.dts | 10 ++++----
drivers/net/phy/dp83867.c | 28 ++++++++++++++++++++++
3 files changed, 46 insertions(+), 4 deletions(-)
--
2.10.0.372.g6fe1b14
^ permalink raw reply
* Coresight ETF trace dump failed in Juno r1 with 4.8-rc8
From: Sudeep Holla @ 2016-10-04 12:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGhh56HHbpLAmsu0z9xMPgq9m0qGBHdr6Kj2xw3vhNYu+fUONA@mail.gmail.com>
On 04/10/16 13:14, Venkatesh Vivekanandan wrote:
> On Tue, Oct 4, 2016 at 4:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 04/10/16 06:37, Venkatesh Vivekanandan wrote:
>>>
>>> On Mon, Oct 3, 2016 at 6:44 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>>
>>>> Hi Venkatesh,
>>>>
>>>> On 03/10/16 12:36, Venkatesh Vivekanandan wrote:
>>>>>
>>>>>
>>>>> Hi All,
>>>>>
>>>>> I am trying to collect ETF trace from Juno R1 and could see "cpu
>>>>> stall" while dumping the trace. Attached is the log of sequence
>>>>> followed. Was trying to collect the trace data from hardware and see
>>>>> if it is any valid data. Am I missing anything here?.
>>>>>
>>>>
>>>> There are few fixes from me and Suzuki queued for v4.9.
>>>> Can you check if this issue persists even on linux-next ?
>>>
>>>
>>> Issue is the same in linux-next as well. Please find the attached log.
>>>
>>
>> OK, what do you mean by the issue still being present. I no longer see
>> any ETM failures or messages. Does the system hang when you try to read
>> /dev/<addr>.etf ?
> Yes, system hangs while reading /dev/<addr>.etf. Most of the times, it
> gives RCU stall and sometimes just hangs and I have to reboot in both
> case.
OK, I am unable reproduce the issue hence suspecting the firmware to be
very old.
>>
>> Also the RCU stalls could be related to CPUIdle. Are you using latest
>> versions of trusted firmware ?
> Firmware is the one that came along with the board. Didn't update to
> anything latest.
>
> From bootlog,
>
> [ 0.000000] Boot CPU: AArch64 Processor [411fd071]
> [ 0.000000] earlycon: pl11 at MMIO 0x000000007ff80000 (options '')
> [ 0.000000] bootconsole [pl11] enabled
> [ 0.000000] efi: Getting EFI parameters from FDT:
> [ 0.000000] efi: EFI v2.50 by ARM Juno EFI Nov 24 2015 17:54:31
> [ 0.000000] efi: ACPI=0xfe720000 ACPI 2.0=0xfe720014 PROP=0xfe773f30
>
No, I was referring to the ones very early in the boot, something like:
NOTICE: Booting Trusted Firmware
NOTICE: BL1: v1.2(release):a5a423100822
NOTICE: BL1: Built : 11:42:57, Sep 30 2016
NOTICE: BL1: Booting BL2
NOTICE: BL2: v1.2(release):a5a423100822
NOTICE: BL2: Built : 11:42:58, Sep 30 2016
NOTICE: BL1: Booting BL31
NOTICE: BL31: v1.2(release):a5a423100822
NOTICE: BL31: Built : 11:43:06, Sep 30 2016
UEFI firmware (version built at 11:42:39 on Sep 30 2016)
Ignore the dates here as I have build one last week.
> Please let me know, if the firmware needs to be updated to specific
> version. Any pointers on that would be appreciated.
>
Yes always better as they will have bug fixes. You can get one from [1]
or just follow instructions @[2].
[1]
http://snapshots.linaro.org/member-builds/armlt-platforms-release/32/juno-uefi.zip
[2] https://community.arm.com/docs/DOC-11395
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH 2/2] tty: serial: Makefile: move kgdb to be initialized last
From: Nicolae Rosia @ 2016-10-04 12:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475585177-28777-1-git-send-email-nicolae.rosia.oss@gmail.com>
From: Nicolae Rosia <nicolae_rosia@mentor.com>
fsl_lpuart cannot be used with kgdb because the kgdb
initcall is called before the driver's init.
Move kgdb to be initialized after all serial drivers
have been inited.
Signed-off-by: Nicolae Rosia <nicolae_rosia@mentor.com>
Signed-off-by: Stefan Golinschi <stefan.golinschi@gmail.com>
---
drivers/tty/serial/Makefile | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index 1278d37..8016ef9 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -62,13 +62,11 @@ obj-$(CONFIG_SERIAL_ATMEL) += atmel_serial.o
obj-$(CONFIG_SERIAL_UARTLITE) += uartlite.o
obj-$(CONFIG_SERIAL_MSM) += msm_serial.o
obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
-obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
obj-$(CONFIG_SERIAL_ST_ASC) += st-asc.o
obj-$(CONFIG_SERIAL_TILEGX) += tilegx.o
-obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o
obj-$(CONFIG_SERIAL_QE) += ucc_uart.o
obj-$(CONFIG_SERIAL_TIMBERDALE) += timbuart.o
obj-$(CONFIG_SERIAL_GRLIB_GAISLER_APBUART) += apbuart.o
@@ -96,3 +94,6 @@ obj-$(CONFIG_SERIAL_MPS2_UART) += mps2-uart.o
# GPIOLIB helpers for modem control lines
obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
+
+obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
+obj-$(CONFIG_KGDB_SERIAL_CONSOLE) += kgdboc.o
--
2.5.5
^ permalink raw reply related
* [PATCH 1/2] tty: serial: fsl_lpuart: add polled console functions
From: Nicolae Rosia @ 2016-10-04 12:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475585177-28777-1-git-send-email-nicolae.rosia.oss@gmail.com>
From: Nicolae Rosia <nicolae_rosia@mentor.com>
This adds polling functions as used by kgdb.
Signed-off-by: Nicolae Rosia <nicolae_rosia@mentor.com>
Signed-off-by: Stefan Golinschi <stefan.golinschi@gmail.com>
---
drivers/tty/serial/fsl_lpuart.c | 66 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index de9d510..49d7526 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -431,6 +431,67 @@ static void lpuart_flush_buffer(struct uart_port *port)
}
}
+#if defined(CONFIG_CONSOLE_POLL)
+
+static int lpuart_poll_init(struct uart_port *port)
+{
+ struct lpuart_port *sport = container_of(port,
+ struct lpuart_port, port);
+ unsigned long flags;
+ unsigned char temp;
+
+ sport->port.fifosize = 0;
+
+ spin_lock_irqsave(&sport->port.lock, flags);
+ /* Disable Rx & Tx */
+ writeb(0, sport->port.membase + UARTCR2);
+
+ temp = readb(sport->port.membase + UARTPFIFO);
+ /* Enable Rx and Tx FIFO */
+ writeb(temp | UARTPFIFO_RXFE | UARTPFIFO_TXFE,
+ sport->port.membase + UARTPFIFO);
+
+ /* flush Tx and Rx FIFO */
+ writeb(UARTCFIFO_TXFLUSH | UARTCFIFO_RXFLUSH,
+ sport->port.membase + UARTCFIFO);
+
+ /* explicitly clear RDRF */
+ if (readb(sport->port.membase + UARTSR1) & UARTSR1_RDRF) {
+ readb(sport->port.membase + UARTDR);
+ writeb(UARTSFIFO_RXUF, sport->port.membase + UARTSFIFO);
+ }
+
+ writeb(0, sport->port.membase + UARTTWFIFO);
+ writeb(1, sport->port.membase + UARTRWFIFO);
+
+ /* Enable Rx and Tx */
+ writeb(UARTCR2_RE | UARTCR2_TE, sport->port.membase + UARTCR2);
+ spin_unlock_irqrestore(&sport->port.lock, flags);
+
+ return 0;
+}
+
+static void lpuart_poll_put_char(struct uart_port *port, unsigned char c)
+{
+ unsigned int status;
+
+ /* drain */
+ while (!(readb(port->membase + UARTSR1) & UARTSR1_TDRE))
+ barrier();
+
+ writeb(c, port->membase + UARTDR);
+}
+
+static int lpuart_poll_get_char(struct uart_port *port)
+{
+ if (!(readb(port->membase + UARTSR1) & UARTSR1_RDRF))
+ return NO_POLL_CHAR;
+
+ return readb(port->membase + UARTDR);
+}
+
+#endif
+
static inline void lpuart_transmit_buffer(struct lpuart_port *sport)
{
struct circ_buf *xmit = &sport->port.state->xmit;
@@ -1596,6 +1657,11 @@ static const struct uart_ops lpuart_pops = {
.config_port = lpuart_config_port,
.verify_port = lpuart_verify_port,
.flush_buffer = lpuart_flush_buffer,
+#if defined(CONFIG_CONSOLE_POLL)
+ .poll_init = lpuart_poll_init,
+ .poll_get_char = lpuart_poll_get_char,
+ .poll_put_char = lpuart_poll_put_char,
+#endif
};
static const struct uart_ops lpuart32_pops = {
--
2.5.5
^ permalink raw reply related
* [PATCH 0/2] tty: serial: kgdb support for fsl_lpuart
From: Nicolae Rosia @ 2016-10-04 12:46 UTC (permalink / raw)
To: linux-arm-kernel
From: Nicolae Rosia <nicolae_rosia@mentor.com>
This patch series enables kgdb to be used with fsl_lpuart
on Vybrid processors.
Tested on Vybrid VF61
Nicolae Rosia (2):
tty: serial: fsl_lpuart: add polled console functions
tty: serial: Makefile: move kgdb to be initialized last
drivers/tty/serial/Makefile | 5 ++--
drivers/tty/serial/fsl_lpuart.c | 66 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 69 insertions(+), 2 deletions(-)
--
2.5.5
^ permalink raw reply
* [PATCH 06/14] ASoC: Add sun8i digital audio codec
From: Thomas Petazzoni @ 2016-10-04 12:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <85cbd9926e52d0aa03f6bbfd8794373d8db491e0.1475571575.git.mylene.josserand@free-electrons.com>
Hello,
On Tue, 4 Oct 2016 11:46:19 +0200, Myl?ne Josserand wrote:
> Add the digital sun8i audio codec which handles the base register
> (without DAI).
I'm not sure what you mean by "which handles the base register".
> diff --git a/sound/soc/sunxi/Kconfig b/sound/soc/sunxi/Kconfig
> index 7aee95a..9e287b0 100644
> --- a/sound/soc/sunxi/Kconfig
> +++ b/sound/soc/sunxi/Kconfig
> @@ -27,6 +27,15 @@ config SND_SUN4I_SPDIF
> Say Y or M to add support for the S/PDIF audio block in the Allwinner
> A10 and affiliated SoCs.
>
> +config SND_SUN8I_CODEC
> + tristate "Allwinner SUN8I audio codec"
> + select REGMAP_MMIO
> + help
Indentation issue here, it should be intended with one tab, not spaces.
You probably also want a "depends on OF" here.
> +/* CODEC_OFFSET represents the offset of the codec registers
> + * and not all the DAI registers
> + */
This is not the proper comment style I believe for audio code, it
should be:
/*
* ...
*/
> +#define CODEC_OFFSET 0x200
Do you really need this CODEC_OFFSET macro? Why not simply use directly
the right offsets? I.e instead of:
#define SUN8I_SYSCLK_CTL (0x20c - CODEC_OFFSET)
use:
#define SUN8I_SYSCLK_CTL 0xc
> +#define CODEC_BASSADDRESS 0x01c22c00
This define is not used anywhere.
> +#define SUN8I_SYSCLK_CTL (0x20c - CODEC_OFFSET)
> +#define SUN8I_SYSCLK_CTL_AIF1CLK_ENA (11)
> +#define SUN8I_SYSCLK_CTL_SYSCLK_ENA (3)
> +#define SUN8I_SYSCLK_CTL_SYSCLK_SRC (0)
Parenthesis around single values are not really useful.
> +#define SUN8I_MOD_CLK_ENA (0x210 - CODEC_OFFSET)
> +#define SUN8I_MOD_CLK_ENA_AIF1 (15)
> +#define SUN8I_MOD_CLK_ENA_DAC (2)
> +#define SUN8I_MOD_RST_CTL (0x214 - CODEC_OFFSET)
> +#define SUN8I_MOD_RST_CTL_AIF1 (15)
> +#define SUN8I_MOD_RST_CTL_DAC (2)
> +#define SUN8I_SYS_SR_CTRL (0x218 - CODEC_OFFSET)
> +#define SUN8I_SYS_SR_CTRL_AIF1_FS (12)
> +#define SUN8I_SYS_SR_CTRL_AIF2_FS (8)
> +#define SUN8I_AIF1CLK_CTRL (0x240 - CODEC_OFFSET)
> +#define SUN8I_AIF1CLK_CTRL_AIF1_MSTR_MOD (15)
> +#define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_INV (14)
> +#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_INV (13)
> +#define SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV (9)
> +#define SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV (6)
> +#define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ (4)
> +#define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT (2)
> +#define SUN8I_AIF1_DACDAT_CTRL (0x248 - CODEC_OFFSET)
> +#define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA (15)
> +#define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA (14)
> +#define SUN8I_DAC_DIG_CTRL (0x320 - CODEC_OFFSET)
> +#define SUN8I_DAC_DIG_CTRL_ENDA (15)
> +#define SUN8I_DAC_MXR_SRC (0x330 - CODEC_OFFSET)
> +#define SUN8I_DAC_MXR_SRC_DACL_MXR_SRC_AIF1DA0L (15)
> +#define SUN8I_DAC_MXR_SRC_DACL_MXR_SRC_AIF1DA1L (14)
> +#define SUN8I_DAC_MXR_SRC_DACL_MXR_SRC_AIF2DACL (13)
> +#define SUN8I_DAC_MXR_SRC_DACL_MXR_SRC_ADCL (12)
> +#define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_AIF1DA0R (11)
> +#define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_AIF1DA1R (10)
> +#define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_AIF2DACR (9)
> +#define SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR (8)
Indentation of the value is not very clean for those last defines.
> +static int sun8i_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
> +{
> + struct sun8i_codec *scodec = snd_soc_codec_get_drvdata(dai->codec);
> + unsigned long value;
I'm not sure "unsigned long" is a very good choice here, it's going to
be a 64 bits integer on 64 bits platform. I'd suggest to use "u32",
which also seems to be what's used in _set_fmt() function of the
sun4i-i2s.c driver.
> +static int sun8i_codec_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *dai)
> +{
> + int rs_value = 0;
Two spaces before the = sign, not needed. Is the initialization to 0
really needed? Also, this should be a u32.
> + regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL,
> + 0x3 << SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ,
Maybe a #define value to replace the hardcoded 0x3 ?
> + rs_value << SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ);
> +
> + /* calculate bclk_lrck_div Ratio */
> + bclk_lrck_div = sample_resolution * 2;
> + switch (bclk_lrck_div) {
> + case 16:
> + bclk_lrck_div = 0;
> + break;
> + case 32:
> + bclk_lrck_div = 1;
> + break;
> + case 64:
> + bclk_lrck_div = 2;
> + break;
> + case 128:
> + bclk_lrck_div = 3;
> + break;
> + case 256:
> + bclk_lrck_div = 4;
> + break;
This could quite easily be replaced by a formula, if you don't care
about error checking:
bclk_lrck_div = log2(bclk_lrck_div) - 4;
Of course, if you care about error checking, this switch is nicer.
> + default:
So there's no error checking if the value is not supported?
> + break;
> + }
> + regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL,
> + 0x7 << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV,
#define to replace the hard-coded 0x7 ?
> + bclk_lrck_div << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV);
> +
> + sample_rate = sun8i_codec_get_hw_rate(params);
> + if (sample_rate < 0)
> + return sample_rate;
> +
> + regmap_update_bits(scodec->regmap, SUN8I_SYS_SR_CTRL,
> + 0xf << SUN8I_SYS_SR_CTRL_AIF1_FS,
Ditto 0xf
> + sample_rate << SUN8I_SYS_SR_CTRL_AIF1_FS);
> + regmap_update_bits(scodec->regmap, SUN8I_SYS_SR_CTRL,
> + 0xf << SUN8I_SYS_SR_CTRL_AIF2_FS,
Ditto 0xf.
> +static struct snd_soc_dai_driver sun8i_codec_dai = {
> + .name = "sun8i",
> + /* playback capabilities */
> + .playback = {
> + .stream_name = "Playback",
> + .channels_min = 1,
> + .channels_max = 2,
> + .rates = SNDRV_PCM_RATE_8000_192000 |
> + SNDRV_PCM_RATE_KNOT,
> + .formats = SNDRV_PCM_FMTBIT_S8 |
> + SNDRV_PCM_FMTBIT_S16_LE |
> + SNDRV_PCM_FMTBIT_S18_3LE |
> + SNDRV_PCM_FMTBIT_S20_3LE |
> + SNDRV_PCM_FMTBIT_S24_LE |
> + SNDRV_PCM_FMTBIT_S32_LE,
> + },
> + /* pcm operations */
> + .ops = &sun8i_codec_dai_ops,
> +};
> +EXPORT_SYMBOL(sun8i_codec_dai);
This EXPORT_SYMBOL looks wrong. First because it doesn't seem to be
used outside of this module. And second because using EXPORT_SYMBOL on
a function defined as static doesn't make much sense, as the "static"
qualifier limits the visibility of the symbol to the current
compilation unit.
> +
> +static int sun8i_soc_probe(struct snd_soc_codec *codec)
> +{
> + return 0;
> +}
> +
> +/* power down chip */
> +static int sun8i_soc_remove(struct snd_soc_codec *codec)
> +{
> + return 0;
> +}
I believe you can remove those stub functions.
> +
> +static struct snd_soc_codec_driver sun8i_soc_codec = {
> + .probe = sun8i_soc_probe,
> + .remove = sun8i_soc_remove,
And remove these.
> + .component_driver = {
> + .dapm_widgets = sun8i_codec_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(sun8i_codec_dapm_widgets),
> + .dapm_routes = sun8i_codec_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(sun8i_codec_dapm_routes),
I'm probably missing something, but in the sun4i-codec.c driver, those
fields are initialized directly in the snd_soc_codec_driver structure,
not in the .component_driver sub-structure.
> +static int sun8i_codec_probe(struct platform_device *pdev)
> +{
> + struct resource *res_base;
> + struct sun8i_codec *scodec;
> + void __iomem *base;
> +
> + scodec = devm_kzalloc(&pdev->dev, sizeof(*scodec), GFP_KERNEL);
> + if (!scodec)
> + return -ENOMEM;
> +
> + scodec->dev = &pdev->dev;
> +
> + /* Get the clocks from the DT */
> + scodec->clk_module = devm_clk_get(&pdev->dev, "codec");
> + if (IS_ERR(scodec->clk_module)) {
> + dev_err(&pdev->dev, "Failed to get the module clock\n");
> + return PTR_ERR(scodec->clk_module);
> + }
> + if (clk_prepare_enable(scodec->clk_module))
> + pr_err("err:open failed;\n");
Grr, pr_err, not good. Plus you want to return with an error from the
probe() function.
> +
> + scodec->clk_apb = devm_clk_get(&pdev->dev, "apb");
> + if (IS_ERR(scodec->clk_apb)) {
> + dev_err(&pdev->dev, "Failed to get the apb clock\n");
> + return PTR_ERR(scodec->clk_apb);
> + }
> + if (clk_prepare_enable(scodec->clk_apb))
> + pr_err("err:open failed;\n");
Ditto. + unprepare/disable the previous clock.
> +
> + /* Get base resources, registers and regmap */
> + res_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "audio");
> + base = devm_ioremap_resource(&pdev->dev, res_base);
> + if (IS_ERR(base)) {
> + dev_err(&pdev->dev, "Failed to map the registers\n");
> + return PTR_ERR(base);
> + }
> +
> + scodec->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &sun8i_codec_regmap_config);
> + if (IS_ERR(scodec->regmap)) {
> + dev_err(&pdev->dev, "Failed to create our regmap\n");
> + return PTR_ERR(scodec->regmap);
> + }
> +
> + /* Set the codec data as driver data */
> + dev_set_drvdata(&pdev->dev, scodec);
Use:
platform_set_drvdata(pdev, scodec)
> +
> + snd_soc_register_codec(&pdev->dev, &sun8i_soc_codec, &sun8i_codec_dai,
> + 1);
That's a matter of taste, but I find the "1" alone on its own line a
bit weird. Maybe move &sun8i_codec_dai on the second line as well. But
again, it's mainly a matter of taste, so Mark might disagree here.
> +
> + return 0;
> +}
> +
> +static int sun8i_codec_remove(struct platform_device *pdev)
> +{
> + struct snd_soc_card *card = platform_get_drvdata(pdev);
> + struct sun8i_codec *scodec = snd_soc_card_get_drvdata(card);
> +
> + snd_soc_unregister_codec(&pdev->dev);
> + clk_disable_unprepare(scodec->clk_module);
> + clk_disable_unprepare(scodec->clk_apb);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sun8i_codec_of_match[] = {
> + { .compatible = "allwinner,sun8i-a33-codec" },
> + { .compatible = "allwinner,sun8i-a23-codec" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, sun8i_codec_of_match);
> +
> +static struct platform_driver sun8i_codec_driver = {
> + .driver = {
> + .name = "sun8i-codec",
> + .owner = THIS_MODULE,
> + .of_match_table = sun8i_codec_of_match,
> + },
> + .probe = sun8i_codec_probe,
> + .remove = sun8i_codec_remove,
> +};
> +module_platform_driver(sun8i_codec_driver);
> +
> +MODULE_DESCRIPTION("Allwinner A33 (sun8i) codec driver");
> +MODULE_AUTHOR("huanxin<huanxin@reuuimllatech.com>");
Space between the name and the e-mail address.
> +MODULE_AUTHOR("Myl?ne Josserand <mylene.josserand@free-electrons.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:sun8i-codec");
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH 07/14] ASoC: Add sun8i audio card
From: Thomas Petazzoni @ 2016-10-04 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <33d641ff43f0c0349cdfa2cdbbfdcdde66205596.1475571575.git.mylene.josserand@free-electrons.com>
Hello,
On Tue, 4 Oct 2016 11:46:20 +0200, Myl?ne Josserand wrote:
> +config SND_SUN8I
> + tristate "Allwinner SUN6I/SUN8I audio card support"
> + select SND_SUN8I_CODEC
> + select SND_SUN4I_I2S
> + select SND_SUN8I_CODEC_ANALOG
> + select REGMAP_MMIO
I believe you need a:
depends on OF
since you're unconditionally using some DT-related functionality in the
driver code.
> +#include <linux/firmware.h>
Do you really need this header file? I don't see anything
firmware-loading related in the driver.
> +static int sun8i_probe(struct platform_device *pdev)
> +{
> + struct snd_soc_dai_link *link = &sun8i_dai_link;
> + struct device_node *np = pdev->dev.of_node;
> + int ret;
> +
> + /* register the soc card */
> + sun8i_card.dev = &pdev->dev;
> +
> + /* Retrieve the audio-codec from DT */
> + link->codec_of_node = of_parse_phandle(np, "allwinner,audio-codec", 0);
Whenever you're using of_parse_phandle(), you must have a corresponding
of_node_put() to release the reference to the Device Tree node. So I
guess this should be done 1/ in the error path of ->probe(), and 2/
during the ->remove() hook.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] dmaengine: coh901318: fix integer overflow when shifting more than 32 places
From: Linus Walleij @ 2016-10-04 12:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475172392.2027.12.camel@perches.com>
On Thu, Sep 29, 2016 at 8:06 PM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2016-09-29 at 18:57 +0100, Colin King wrote:
>> Currently U300_DMA_CHANNELS is set to 40, meaning that the shift of 1 can
>> be more than 32 places, which leads to a 32 bit integer overflow. Fix this
>> by casting 1 to a u64 (the same type as started_channels) before shifting
>> it.
>
> trivia:
>
>> diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
> []
>> @@ -1353,7 +1353,7 @@ static ssize_t coh901318_debugfs_read(struct file *file, char __user *buf,
>> tmp += sprintf(tmp, "DMA -- enabled dma channels\n");
>>
>> for (i = 0; i < U300_DMA_CHANNELS; i++)
>> - if (started_channels & (1 << i))
>> + if (started_channels & ((u64)1 << i))
>
> Using
>
> if (started_channels & (1ULL << i))
>
> would be more common.
Even better (IMO):
#include <linux/bitops.h>
if (started_channels & BIT(i))
Apparently code is there to avoid the bit 31 problem, mea culpa.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset
From: Code Kipper @ 2016-10-04 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2c5abe6578c8e4e841cb59357d88ce397551a593.1475571575.git.mylene.josserand@free-electrons.com>
On 4 October 2016 at 11:46, Myl?ne Josserand
<mylene.josserand@free-electrons.com> wrote:
> Add APB deassert function for sun4i-i2s driver.
>
> Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
> ---
> sound/soc/sunxi/sun4i-i2s.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index 687a8f8..f3f7026 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -17,6 +17,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> +#include <linux/reset.h>
>
> #include <sound/dmaengine_pcm.h>
> #include <sound/pcm_params.h>
> @@ -589,6 +590,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
> {
> struct sun4i_i2s *i2s;
> struct resource *res;
> + struct reset_control *reset_apb;
> void __iomem *regs;
> int irq, ret;
>
> @@ -626,7 +628,19 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "Can't get our mod clock\n");
> return PTR_ERR(i2s->mod_clk);
> }
> -
> +
> + reset_apb = devm_reset_control_get(&pdev->dev, "apb_reset");
> + if (IS_ERR(reset_apb)) {
> + dev_err(&pdev->dev, "Can't get apb reset\n");
> + return PTR_ERR(i2s->mod_clk);
> + }
> +
> + ret = reset_control_deassert(reset_apb);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Can't deassert apb reset (%d)\n", ret);
> + return ret;
> + }
> +
Is this functionality only required for A31 and onwards?,
CK
> i2s->playback_dma_data.addr = res->start + SUN4I_I2S_FIFO_TX_REG;
> i2s->playback_dma_data.maxburst = 4;
>
> --
> 2.9.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset
From: Thomas Petazzoni @ 2016-10-04 12:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2c5abe6578c8e4e841cb59357d88ce397551a593.1475571575.git.mylene.josserand@free-electrons.com>
Hello,
On Tue, 4 Oct 2016 11:46:16 +0200, Myl?ne Josserand wrote:
> #include <sound/dmaengine_pcm.h>
> #include <sound/pcm_params.h>
> @@ -589,6 +590,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
> {
> struct sun4i_i2s *i2s;
> struct resource *res;
> + struct reset_control *reset_apb;
> void __iomem *regs;
> int irq, ret;
>
> @@ -626,7 +628,19 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "Can't get our mod clock\n");
> return PTR_ERR(i2s->mod_clk);
> }
> -
> +
> + reset_apb = devm_reset_control_get(&pdev->dev, "apb_reset");
I believe this is a change in the Device Tree binding, since you're
adding support for a new resource. Perhaps the Device Tree binding
documentation should be updated accordingly?
> + if (IS_ERR(reset_apb)) {
> + dev_err(&pdev->dev, "Can't get apb reset\n");
> + return PTR_ERR(i2s->mod_clk);
This should be:
return PTR_ERR(reset_apb);
> + }
> +
> + ret = reset_control_deassert(reset_apb);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Can't deassert apb reset (%d)\n", ret);
> + return ret;
> + }
Do you need to re-assert the reset line in the ->remove() hook?
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Coresight ETF trace dump failed in Juno r1 with 4.8-rc8
From: Venkatesh Vivekanandan @ 2016-10-04 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cc531f08-d09b-1976-90c2-377a5878b1eb@arm.com>
On Tue, Oct 4, 2016 at 4:00 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 04/10/16 06:37, Venkatesh Vivekanandan wrote:
>>
>> On Mon, Oct 3, 2016 at 6:44 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>> Hi Venkatesh,
>>>
>>> On 03/10/16 12:36, Venkatesh Vivekanandan wrote:
>>>>
>>>>
>>>> Hi All,
>>>>
>>>> I am trying to collect ETF trace from Juno R1 and could see "cpu
>>>> stall" while dumping the trace. Attached is the log of sequence
>>>> followed. Was trying to collect the trace data from hardware and see
>>>> if it is any valid data. Am I missing anything here?.
>>>>
>>>
>>> There are few fixes from me and Suzuki queued for v4.9.
>>> Can you check if this issue persists even on linux-next ?
>>
>>
>> Issue is the same in linux-next as well. Please find the attached log.
>>
>
> OK, what do you mean by the issue still being present. I no longer see
> any ETM failures or messages. Does the system hang when you try to read
> /dev/<addr>.etf ?
Yes, system hangs while reading /dev/<addr>.etf. Most of the times, it
gives RCU stall and sometimes just hangs and I have to reboot in both
case.
>
> Also the RCU stalls could be related to CPUIdle. Are you using latest
> versions of trusted firmware ?
Firmware is the one that came along with the board. Didn't update to
anything latest.
>From bootlog,
[ 0.000000] Boot CPU: AArch64 Processor [411fd071]
[ 0.000000] earlycon: pl11 at MMIO 0x000000007ff80000 (options '')
[ 0.000000] bootconsole [pl11] enabled
[ 0.000000] efi: Getting EFI parameters from FDT:
[ 0.000000] efi: EFI v2.50 by ARM Juno EFI Nov 24 2015 17:54:31
[ 0.000000] efi: ACPI=0xfe720000 ACPI 2.0=0xfe720014 PROP=0xfe773f30
Please let me know, if the firmware needs to be updated to specific
version. Any pointers on that would be appreciated.
>
> --
> Regards,
> Sudeep
^ permalink raw reply
* [PATCH 2/2] mfd: ab8500-debugfs: remove unused function
From: Linus Walleij @ 2016-10-04 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1474695413-30460-1-git-send-email-baoyou.xie@linaro.org>
On Sat, Sep 24, 2016 at 7:36 AM, Baoyou Xie <baoyou.xie@linaro.org> wrote:
> We get 1 warning when building kernel with W=1:
> drivers/mfd/ab8500-debugfs.c:1395:6: warning: no previous prototype for 'ab8500_dump_all_banks_to_mem' [-Wmissing-prototypes]
>
> In fact, this function is called by no one and not exported,
> so this patch removes it.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 02/14] clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to ac-dig
From: Thomas Petazzoni @ 2016-10-04 12:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8dfe0935d35b8a218ebf39d37113e27289a0de9b.1475571575.git.mylene.josserand@free-electrons.com>
Hello,
On Tue, 4 Oct 2016 11:46:15 +0200, Myl?ne Josserand wrote:
> Add the flag CLK_SET_RATE_PARENT to 'ac-dig' clock.
There is no need to repeat the commit title inside the commit log
itself. What would be more useful is to explain *why* this is needed.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH 01/14] dma: sun6i-dma: Add burst case of 4
From: Thomas Petazzoni @ 2016-10-04 12:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161004124011.d7f5754a082d5f17d5185fc4@free.fr>
Hello,
On Tue, 4 Oct 2016 12:40:11 +0200, Jean-Francois Moine wrote:
> > Add the case of a burst of 4 which is handled by the SoC.
> >
> > Signed-off-by: Myl?ne Josserand <mylene.josserand@free-electrons.com>
> > ---
> > drivers/dma/sun6i-dma.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 8346199..0485204 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
> > switch (maxburst) {
> > case 1:
> > return 0;
> > + case 4:
> > + return 1;
> > case 8:
> > return 2;
> > default:
> > --
> > 2.9.3
>
> This patch has already been rejected by Maxime in the threads
> http://www.spinics.net/lists/dmaengine/msg08610.html
> and
> http://www.spinics.net/lists/dmaengine/msg08719.html
>
> I hope you will find the way he wants for this maxburst to be added.
I was about to reply to Mylene's e-mail, suggesting that she should add
a comment in the code (and maybe in the commit log) to explain why this
addition is needed, and also that even though the schematics say that
value "1" (max burst size of 4 bytes) is reserved, it is in fact
incorrect. The Allwinner BSP code is really using this value, and it's
the value that makes audio work, so we believe the datasheet is simply
incorrect.
We already discussed it with Maxime, so I believe he should agree this
time. But I would suggest to have such details explained in the commit
log and in a comment in the code.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v3 2/8] scpi: Add alternative legacy structures, functions and macros
From: Neil Armstrong @ 2016-10-04 12:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <da2f9471-4b54-c4cd-c3c2-de09fca52ab6@arm.com>
On 09/19/2016 05:24 PM, Sudeep Holla wrote:
>
>
> On 07/09/16 16:34, Neil Armstrong wrote:
>> In order to support the legacy SCPI protocol variant, add back the structures
>> and macros that varies against the final specification.
>> Add indirection table for legacy commands.
>> Add bitmap field for channel selection
>> Add support for legacy in scpi_send_message.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> drivers/firmware/arm_scpi.c | 218 ++++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 211 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
>> index 9a87687..9ba1020 100644
>> --- a/drivers/firmware/arm_scpi.c
>> +++ b/drivers/firmware/arm_scpi.c
>
> [..]
>
>> @@ -336,6 +424,39 @@ static void scpi_handle_remote_msg(struct mbox_client *c, void *msg)
>> scpi_process_cmd(ch, cmd);
>> }
>>
>> +static void legacy_scpi_process_cmd(struct scpi_chan *ch)
>> +{
>> + unsigned long flags;
>> + struct scpi_xfer *t;
>> +
>> + spin_lock_irqsave(&ch->rx_lock, flags);
>> + if (list_empty(&ch->rx_pending)) {
>> + spin_unlock_irqrestore(&ch->rx_lock, flags);
>> + return;
>> + }
>> +
>> + t = list_first_entry(&ch->rx_pending, struct scpi_xfer, node);
>> + list_del(&t->node);
>> +
>
> This is a bad assumption that it will be always first. The legacy SCPI
> did support multiple commands at a time and they can be reordered when
> SCP responds to them. Except this it's almost same scpi_process_cmd. You
> should be able to use it as is if you pass the command.
I would be happy this was the case...
>
>> + /* check if wait_for_completion is in progress or timed-out */
>> + if (t && !completion_done(&t->done)) {
>> + struct legacy_scpi_shared_mem *mem = ch->rx_payload;
>> + unsigned int len = t->rx_len;
>> +
>> + t->status = le32_to_cpu(mem->status);
>> + memcpy_fromio(t->rx_buf, mem->payload, len);
>> + complete(&t->done);
>> + }
>> + spin_unlock_irqrestore(&ch->rx_lock, flags);
>> +}
>> +
>> +static void legacy_scpi_handle_remote_msg(struct mbox_client *c, void *_msg)
>> +{
>> + struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
>> +
>> + legacy_scpi_process_cmd(ch);
>
> You will get the command in *_msg IIRC. So you can just pass that to
> scpi_process_cmd. You can even reuse scpi_handle_remote_msg
But Amlogic SCP firmware does not answer the command but only the first bit...
so we cannot queue commands because we cannot find back the queued command
from the replied MHU STAT value.
>
> diff --git i/drivers/firmware/arm_scpi.c w/drivers/firmware/arm_scpi.c
> index edf1a3327041..165f2fc3b627 100644
> --- i/drivers/firmware/arm_scpi.c
> +++ w/drivers/firmware/arm_scpi.c
> @@ -419,7 +419,12 @@ static void scpi_handle_remote_msg(struct mbox_client *c, void *msg)
> {
> struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
> struct scpi_shared_mem *mem = ch->rx_payload;
> - u32 cmd = le32_to_cpu(mem->command);
> + u32 cmd;
> +
> + if (ch->is_legacy)
> + cmd = *(u32 *)msg;
> + else
> + cmd = le32_to_cpu(mem->command);
>
> scpi_process_cmd(ch, cmd);
> }
>
>> +}
>> +
>> static void scpi_tx_prepare(struct mbox_client *c, void *msg)
>> {
>> unsigned long flags;
>> @@ -356,6 +477,21 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg)
>> mem->command = cpu_to_le32(t->cmd);
>> }
>>
>> +static void legacy_scpi_tx_prepare(struct mbox_client *c, void *msg)
>> +{
>> + unsigned long flags;
>> + struct scpi_xfer *t = msg;
>> + struct scpi_chan *ch = container_of(c, struct scpi_chan, cl);
>> +
>> + if (t->tx_buf)
>> + memcpy_toio(ch->tx_payload, t->tx_buf, t->tx_len);
>> + if (t->rx_buf) {
>> + spin_lock_irqsave(&ch->rx_lock, flags);
>> + list_add_tail(&t->node, &ch->rx_pending);
>> + spin_unlock_irqrestore(&ch->rx_lock, flags);
>> + }
>> +}
>
> Again here the only difference is token addition. I think we should
> retain that as it's helpful in debugging and I don't think it will have
> any issues. Worst case we can make it conditional but let's check if we
> can retain it first.
Yes token addition works.
>
>> @@ -386,15 +522,25 @@ static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len,
>> struct scpi_xfer *msg;
>> struct scpi_chan *scpi_chan;
>>
>> - chan = atomic_inc_return(&scpi_info->next_chan) % scpi_info->num_chans;
>> + if (scpi_info->is_legacy)
>> + chan = test_bit(cmd, scpi_info->cmd_priority) ? 1 : 0;
>> + else
>> + chan = atomic_inc_return(&scpi_info->next_chan) %
>> + scpi_info->num_chans;
>> scpi_chan = scpi_info->channels + chan;
>>
>> msg = get_scpi_xfer(scpi_chan);
>> if (!msg)
>> return -ENOMEM;
>>
>> - msg->slot = BIT(SCPI_SLOT);
>> - msg->cmd = PACK_SCPI_CMD(cmd, tx_len);
>> + if (scpi_info->is_legacy) {
>> + mutex_lock(&scpi_chan->xfers_lock);
>
> Why does legacy need a different locking scheme ?
Since we cannot queue, locking seems a really good idea...
>
> [...]
>
>> @@ -635,6 +804,24 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val)
>> return ret;
>> }
>>
>> +static int legacy_scpi_sensor_get_value(u16 sensor, u64 *val)
>> +{
>> + __le16 id = cpu_to_le16(sensor);
>> + struct sensor_value buf;
>> + int ret;
>> +
>> + ret = check_cmd(CMD_SENSOR_VALUE);
>> + if (ret)
>> + return ret;
>> +
>> + ret = scpi_send_message(scpi_info->scpi_cmds[CMD_SENSOR_VALUE],
>> + &id, sizeof(id), &buf, sizeof(buf));
>> + if (!ret)
>> + *val = (u64)le32_to_cpu(buf.lo_val);
>> +
>
> This is not needed as it's backward compatible as discussed before.
> Any particular reason you retained it here ?
>
Sudeep,
I merged the commands as asked but Amlogic's SCP firmware only replies the value 1 in the MHU STAT registers.
This implies that :
- We cannot distinguish what command ended in scpi_handle_remote_msg
- We cannot find the last rx command in rx_pending list
- We cannot read rx data length from the replied command
- We cannot push multiple commands
- We also need to wait for TX commands completion
- We need locking in scpi_send_message around mbox_send_message and completion
I have an highly tweaked version with simplified path for legacy but with merged handle_remote_msg, tx_prepare and send_message functions
I will post shortly.
Neil
^ permalink raw reply
* PROBLEM: DWC3 USB 3.0 not working on Odroid-XU4 with Exynos 5422
From: Vivek Gautam @ 2016-10-04 12:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1475578687.1912.28.camel@mniewoehner.de>
Hi Michael,
On Tue, Oct 4, 2016 at 4:28 PM, Michael Niew?hner <linux@mniewoehner.de> wrote:
> > > > > > [1.] One line summary of the problem:
> > > > > > DWC3 USB 3.0 not working on Odroid-XU4 with Exynos 5422
> > > > > >
> > > > > > [2.] Full description of the problem/report:
> > > > > > No usb 3.0 devices are being detected when attached while USB 2.0
> > > > > > devices work on the same port.
> > > > > > USB 3.0 works after applying patches [9.1] and [9.2], but seems
> > > > > > to be
> > > > > > buggy. The usb hub is redetected every time an usb device is
> > > > > > attached.
[snip]
>> > > > > > [9.] Other notes, patches, fixes, workarounds:
>> > > > > > [9.1] https://lkml.org/lkml/2014/4/28/234
>> > > > > > [9.2] https://lkml.org/lkml/2015/2/2/259
>>
>> These patches are required to get USB super-speed working on Exynos5420/5800.
>> But they did not make to upstream. There was resistance on adding new
>> phy_calibrate()
>> callback.
>>
>> Without these patches the Exynos5420/5800 will enumerate all
>> super-speed capable devices
>> as high-speed devices.
>> Last time we checked with exynos542x smdk boards and peach-* boards,
>> we could get the
>> Super - speed devices working. I have not tested odroid anytime so
>> don't have much idea about the
>> its intricacies.
>> I guess Anand was able to use these patches to get his kernel working in past.
>
>
> The patches don't work anymore with 4.8-rc* / 4.8. They worked - but very
> unstable - with 4.7.
>
> One more problem appeared since one of the 4.8-RCs: reboot hangs when the dwc3
> module is loaded. If I unload it before reboot / shutdown everything is fine.
>
>
>>
>> When you have a downstream on-board usb hub, ideally it should be able
>> to detect the devices
>> and not reset everytime you connect a new device (like you mentioned earlier).
>> There can be two possible reasons why the hub keeps getting reset ever
>> after applying the above
>> mentioned patches:
>> 1) the clock rates are not proper.
>> 2) the regulator load setting is not enough to drive the hub.
>>
>> Anand, can you please point Michael to an older kernel with which you
>> could test usb on odroid successfully ?
>> You can compare the clocks with an older version and see if there'a
>> any difference.
>>
>> Any possibility of any other framework (such as, bus-freq) trimming
>> down the clock - rates ?
>
>
> ################################
> # v4.7.5
> ################################
>
> $ cat /sys/kernel/debug/clk/clk_summary | grep usb
> sclk_usbh20_scan_clk 0 0 480000000 0
> sclk_usbh20 0 0 48000
> 000 0
> mout_usbd300 1 1 24000000 0
> dout_usbd300 0 0 24000000 0
> sclk_usbd300
> 0 0 24000000 0
> dout_usbphy300 1 1 24000000 0
> sclk_usbphy300 4 4 24000
> 000 0
> mout_usbd301 1 1 24000000 0
> dout_usbd301 0 0 24000000 0
> sclk_usbd301
> 0 0 24000000 0
> dout_usbphy301 1 1 24000000 0
> sclk_usbphy301 3 3 24000
> 000 0
> usbd301 1 1 100000000
> usbd300 1 1 100000000
>
> usbh20 3 3 100000000 0
>
> ################################
> # v4.8.0
> ################################
>
> $ cat /sys/kernel/debug/clk/clk_summary | grep usb
> sclk_usbh20_scan_clk 0 0 480000000 0
> sclk_usbh20 0 0 48000000
> 0
> mout_usbd300 1 1 24000000 0
> dout_usbd300 0 0 24000000 0
> sclk_usbd300
> 0 0 24000000 0
> dout_usbphy300 1 1 24000000 0
> sclk_usbphy300 4 4 24000000
> 0
> mout_usbd301 1 1 24000000 0
> dout_usbd301 0 0 24000000 0
> sclk_usbd301
> 0 0 24000000 0
> dout_usbphy301 1 1 24000000 0
> sclk_usbphy301 3 3 24000000
> 0
> usbd301 1 1 100000000
This clock should have been 200MHz.
> usbd300 1 1 100000000
> usbh2
> 0 3 3 100000000 0
>
> $ cat /sys/kernel/debug/usb/devices
> <<system hangs>>
>
The clocks are same across working/non-working.
Is it possible to bisect the commit that's causing hang for 4.8x ?
Adding few of the folks from Samsung who can test dwc3 usb on smdk/peach boards.
+Alim, Pankaj
Hi Alim, Pankaj,
can you please give a try with 4.8 kernel on peach/smdk542x board and
see if dwc3 usb works or not.
You may need to the patches mentioned in [9.1] and [9.2] mentioned above.
[snip]
Thanks
Vivek
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox