Re: [PATCH v7 0/3] vduse: add support for networking devices
Hello Michael, On 2/1/24 09:40, Michael S. Tsirkin wrote: On Thu, Feb 01, 2024 at 09:34:11AM +0100, Maxime Coquelin wrote: Hi Jason, It looks like all patches got acked by you. Any blocker to queue the series for next release? Thanks, Maxime I think it's good enough at this point. Will put it in linux-next shortly. I fetched linux-next and it seems the series is not in yet. Is there anything to be reworked on my side? Thanks, Maxime
Re: [PATCH] vduse: implement DMA sync callbacks
Hello Christoph,
On 2/20/24 10:01, Christoph Hellwig wrote:
On Mon, Feb 19, 2024 at 06:06:06PM +0100, Maxime Coquelin wrote:
Since commit 295525e29a5b ("virtio_net: merge dma
operations when filling mergeable buffers"), VDUSE device
require support for DMA's .sync_single_for_cpu() operation
as the memory is non-coherent between the device and CPU
because of the use of a bounce buffer.
This patch implements both .sync_single_for_cpu() and
sync_single_for_device() callbacks, and also skip bounce
buffer copies during DMA map and unmap operations if the
DMA_ATTR_SKIP_CPU_SYNC attribute is set to avoid extra
copies of the same buffer.
vduse really needs to get out of implementing fake DMA operations for
something that is not DMA.
I wasn't involved when vduse was initially submitted, I don't know if
any alternative was considered at that time.
Do you have something specific in mind we could do to have VDUSE not
implementing DMA ops, knowing other vDPA devices rely on DMA?
Thanks,
Maxime
[PATCH] vduse: implement DMA sync callbacks
Since commit 295525e29a5b ("virtio_net: merge dma
operations when filling mergeable buffers"), VDUSE device
require support for DMA's .sync_single_for_cpu() operation
as the memory is non-coherent between the device and CPU
because of the use of a bounce buffer.
This patch implements both .sync_single_for_cpu() and
.sync_single_for_device() callbacks, and also skip bounce
buffer copies during DMA map and unmap operations if the
DMA_ATTR_SKIP_CPU_SYNC attribute is set to avoid extra
copies of the same buffer.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/iova_domain.c | 27 ---
drivers/vdpa/vdpa_user/iova_domain.h | 8
drivers/vdpa/vdpa_user/vduse_dev.c | 22 ++
3 files changed, 54 insertions(+), 3 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/iova_domain.c
b/drivers/vdpa/vdpa_user/iova_domain.c
index 5e4a77b9bae6..791d38d6284c 100644
--- a/drivers/vdpa/vdpa_user/iova_domain.c
+++ b/drivers/vdpa/vdpa_user/iova_domain.c
@@ -373,6 +373,26 @@ static void vduse_domain_free_iova(struct iova_domain
*iovad,
free_iova_fast(iovad, iova >> shift, iova_len);
}
+void vduse_domain_sync_single_for_device(struct vduse_iova_domain *domain,
+ dma_addr_t dma_addr, size_t size,
+ enum dma_data_direction dir)
+{
+ read_lock(&domain->bounce_lock);
+ if (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL)
+ vduse_domain_bounce(domain, dma_addr, size, DMA_TO_DEVICE);
+ read_unlock(&domain->bounce_lock);
+}
+
+void vduse_domain_sync_single_for_cpu(struct vduse_iova_domain *domain,
+ dma_addr_t dma_addr, size_t size,
+ enum dma_data_direction dir)
+{
+ read_lock(&domain->bounce_lock);
+ if (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL)
+ vduse_domain_bounce(domain, dma_addr, size, DMA_FROM_DEVICE);
+ read_unlock(&domain->bounce_lock);
+}
+
dma_addr_t vduse_domain_map_page(struct vduse_iova_domain *domain,
struct page *page, unsigned long offset,
size_t size, enum dma_data_direction dir,
@@ -393,7 +413,8 @@ dma_addr_t vduse_domain_map_page(struct vduse_iova_domain
*domain,
if (vduse_domain_map_bounce_page(domain, (u64)iova, (u64)size, pa))
goto err_unlock;
- if (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL)
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
+ (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL))
vduse_domain_bounce(domain, iova, size, DMA_TO_DEVICE);
read_unlock(&domain->bounce_lock);
@@ -411,9 +432,9 @@ void vduse_domain_unmap_page(struct vduse_iova_domain
*domain,
enum dma_data_direction dir, unsigned long attrs)
{
struct iova_domain *iovad = &domain->stream_iovad;
-
read_lock(&domain->bounce_lock);
- if (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL)
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
+ (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL))
vduse_domain_bounce(domain, dma_addr, size, DMA_FROM_DEVICE);
vduse_domain_unmap_bounce_page(domain, (u64)dma_addr, (u64)size);
diff --git a/drivers/vdpa/vdpa_user/iova_domain.h
b/drivers/vdpa/vdpa_user/iova_domain.h
index 173e979b84a9..f92f22a7267d 100644
--- a/drivers/vdpa/vdpa_user/iova_domain.h
+++ b/drivers/vdpa/vdpa_user/iova_domain.h
@@ -44,6 +44,14 @@ int vduse_domain_set_map(struct vduse_iova_domain *domain,
void vduse_domain_clear_map(struct vduse_iova_domain *domain,
struct vhost_iotlb *iotlb);
+void vduse_domain_sync_single_for_device(struct vduse_iova_domain *domain,
+ dma_addr_t dma_addr, size_t size,
+ enum dma_data_direction dir);
+
+void vduse_domain_sync_single_for_cpu(struct vduse_iova_domain *domain,
+ dma_addr_t dma_addr, size_t size,
+ enum dma_data_direction dir);
+
dma_addr_t vduse_domain_map_page(struct vduse_iova_domain *domain,
struct page *page, unsigned long offset,
size_t size, enum dma_data_direction dir,
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 1d24da79c399..75354ce186a1 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -798,6 +798,26 @@ static const struct vdpa_config_ops vduse_vdpa_config_ops
= {
.free = vduse_vdpa_free,
};
+static void vduse_dev_sync_single_for_device(struct device *dev,
+dma_addr_t d
Re: [PATCH v7 0/3] vduse: add support for networking devices
On 2/1/24 09:40, Michael S. Tsirkin wrote: On Thu, Feb 01, 2024 at 09:34:11AM +0100, Maxime Coquelin wrote: Hi Jason, It looks like all patches got acked by you. Any blocker to queue the series for next release? Thanks, Maxime I think it's good enough at this point. Will put it in linux-next shortly. Thanks Michael!
Re: [PATCH v7 0/3] vduse: add support for networking devices
Hi Jason, It looks like all patches got acked by you. Any blocker to queue the series for next release? Thanks, Maxime On 1/9/24 12:10, Maxime Coquelin wrote: This small series enables virtio-net device type in VDUSE. With it, basic operation have been tested, both with virtio-vdpa and vhost-vdpa using DPDK Vhost library series adding VDUSE support using split rings layout (merged in DPDK v23.07-rc1). Control queue support (and so multiqueue) has also been tested, but requires a Kernel series from Jason Wang relaxing control queue polling [1] to function reliably, so while Jason rework is done, a patch is added to fail init if control queue feature is requested.(tested also with DPDK v23.11). Changes in v7: == - Fail init only if VIRTIO_NET_F_CTRL_VQ is requested. - Convert to use BIT_ULL() macro - Rebased Changes in v6: == - Remove SELinux support from the series, will be handled in a dedicated one. - Require CAP_NET_ADMIN for Net devices creation (Jason). - Fail init if control queue features are requested for Net device type (Jason). - Rebased on latest master. Changes in v5: == - Move control queue disablement patch before Net devices enablement (Jason). - Unify operations LSM hooks into a single hook. - Rebase on latest master. Maxime Coquelin (3): vduse: validate block features only with block devices vduse: Temporarily fail if control queue feature requested vduse: enable Virtio-net device type drivers/vdpa/vdpa_user/vduse_dev.c | 24 1 file changed, 20 insertions(+), 4 deletions(-)
[PATCH v7 3/3] vduse: enable Virtio-net device type
This patch adds Virtio-net device type to the supported
devices types.
Initialization fails if the device does not support
VIRTIO_F_VERSION_1 feature, in order to guarantee the
configuration space is read-only. It also fails with
-EPERM if the CAP_NET_ADMIN is missing.
Acked-by: Jason Wang
Reviewed-by: Eugenio Pérez
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 00f3f562ab5d..8924bbc55635 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -143,6 +143,7 @@ static struct workqueue_struct *vduse_irq_bound_wq;
static u32 allowed_device_id[] = {
VIRTIO_ID_BLOCK,
+ VIRTIO_ID_NET,
};
static inline struct vduse_dev *vdpa_to_vduse(struct vdpa_device *vdpa)
@@ -1686,6 +1687,10 @@ static bool features_is_valid(struct vduse_dev_config
*config)
(config->features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ)))
return false;
+ if ((config->device_id == VIRTIO_ID_NET) &&
+ !(config->features & BIT_ULL(VIRTIO_F_VERSION_1)))
+ return false;
+
return true;
}
@@ -1793,6 +1798,10 @@ static int vduse_create_dev(struct vduse_dev_config
*config,
int ret;
struct vduse_dev *dev;
+ ret = -EPERM;
+ if ((config->device_id == VIRTIO_ID_NET) && !capable(CAP_NET_ADMIN))
+ goto err;
+
ret = -EEXIST;
if (vduse_find_dev(config->name))
goto err;
@@ -2036,6 +2045,7 @@ static const struct vdpa_mgmtdev_ops vdpa_dev_mgmtdev_ops
= {
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_BLOCK, VIRTIO_DEV_ANY_ID },
+ { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
};
--
2.43.0
[PATCH v7 2/3] vduse: Temporarily fail if control queue feature requested
Virtio-net driver control queue implementation is not safe when used with VDUSE. If the VDUSE application does not reply to control queue messages, it currently ends up hanging the kernel thread sending this command. Some work is on-going to make the control queue implementation robust with VDUSE. Until it is completed, let's fail features check if control-queue feature is requested. Signed-off-by: Maxime Coquelin --- drivers/vdpa/vdpa_user/vduse_dev.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c index a5af6d4077b8..00f3f562ab5d 100644 --- a/drivers/vdpa/vdpa_user/vduse_dev.c +++ b/drivers/vdpa/vdpa_user/vduse_dev.c @@ -8,6 +8,7 @@ * */ +#include "linux/virtio_net.h" #include #include #include @@ -28,6 +29,7 @@ #include #include #include +#include #include #include "iova_domain.h" @@ -1680,6 +1682,9 @@ static bool features_is_valid(struct vduse_dev_config *config) if ((config->device_id == VIRTIO_ID_BLOCK) && (config->features & BIT_ULL(VIRTIO_BLK_F_CONFIG_WCE))) return false; + else if ((config->device_id == VIRTIO_ID_NET) && + (config->features & BIT_ULL(VIRTIO_NET_F_CTRL_VQ))) + return false; return true; } -- 2.43.0
[PATCH v7 1/3] vduse: validate block features only with block devices
This patch is preliminary work to enable network device
type support to VDUSE.
As VIRTIO_BLK_F_CONFIG_WCE shares the same value as
VIRTIO_NET_F_HOST_TSO4, we need to restrict its check
to Virtio-blk device type.
Acked-by: Jason Wang
Reviewed-by: Xie Yongji
Reviewed-by: Eugenio Pérez
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0ddd4b8abecb..a5af6d4077b8 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1671,13 +1671,14 @@ static bool device_is_allowed(u32 device_id)
return false;
}
-static bool features_is_valid(u64 features)
+static bool features_is_valid(struct vduse_dev_config *config)
{
- if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
+ if (!(config->features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)))
return false;
/* Now we only support read-only configuration space */
- if (features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE))
+ if ((config->device_id == VIRTIO_ID_BLOCK) &&
+ (config->features & BIT_ULL(VIRTIO_BLK_F_CONFIG_WCE)))
return false;
return true;
@@ -1704,7 +1705,7 @@ static bool vduse_validate_config(struct vduse_dev_config
*config)
if (!device_is_allowed(config->device_id))
return false;
- if (!features_is_valid(config->features))
+ if (!features_is_valid(config))
return false;
return true;
--
2.43.0
[PATCH v7 0/3] vduse: add support for networking devices
This small series enables virtio-net device type in VDUSE. With it, basic operation have been tested, both with virtio-vdpa and vhost-vdpa using DPDK Vhost library series adding VDUSE support using split rings layout (merged in DPDK v23.07-rc1). Control queue support (and so multiqueue) has also been tested, but requires a Kernel series from Jason Wang relaxing control queue polling [1] to function reliably, so while Jason rework is done, a patch is added to fail init if control queue feature is requested.(tested also with DPDK v23.11). Changes in v7: == - Fail init only if VIRTIO_NET_F_CTRL_VQ is requested. - Convert to use BIT_ULL() macro - Rebased Changes in v6: == - Remove SELinux support from the series, will be handled in a dedicated one. - Require CAP_NET_ADMIN for Net devices creation (Jason). - Fail init if control queue features are requested for Net device type (Jason). - Rebased on latest master. Changes in v5: == - Move control queue disablement patch before Net devices enablement (Jason). - Unify operations LSM hooks into a single hook. - Rebase on latest master. Maxime Coquelin (3): vduse: validate block features only with block devices vduse: Temporarily fail if control queue feature requested vduse: enable Virtio-net device type drivers/vdpa/vdpa_user/vduse_dev.c | 24 1 file changed, 20 insertions(+), 4 deletions(-) -- 2.43.0
Re: [PATCH v6 2/3] vduse: Temporarily fail if control queue features requested
On 1/5/24 10:59, Eugenio Perez Martin wrote:
On Fri, Jan 5, 2024 at 9:12 AM Maxime Coquelin
wrote:
On 1/5/24 03:45, Jason Wang wrote:
On Thu, Jan 4, 2024 at 11:38 PM Maxime Coquelin
wrote:
Virtio-net driver control queue implementation is not safe
when used with VDUSE. If the VDUSE application does not
reply to control queue messages, it currently ends up
hanging the kernel thread sending this command.
Some work is on-going to make the control queue
implementation robust with VDUSE. Until it is completed,
let's fail features check if any control-queue related
feature is requested.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0486ff672408..94f54ea2eb06 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include "iova_domain.h"
@@ -46,6 +47,15 @@
#define IRQ_UNBOUND -1
+#define VDUSE_NET_INVALID_FEATURES_MASK \
+ (BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |\
+BIT_ULL(VIRTIO_NET_F_CTRL_RX) | \
+BIT_ULL(VIRTIO_NET_F_CTRL_VLAN) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_ANNOUNCE) | \
+BIT_ULL(VIRTIO_NET_F_MQ) | \
+BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR) | \
+BIT_ULL(VIRTIO_NET_F_RSS))
We need to make this as well:
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
I missed it, and see others have been added in the Virtio spec
repository (BTW, I see this specific one is missing in the dependency
list [0], I will submit a patch).
I wonder if it is not just simpler to just check for
VIRTIO_NET_F_CTRL_VQ is requested. As we fail instead of masking out,
the VDUSE driver won't be the one violating the spec so it should be
good?
It will avoid having to update the mask if new features depending on it
are added (or forgetting to update it).
WDYT?
I think it is safer to work with a whitelist, instead of a blacklist.
As any new feature might require code changes in QEMU. Is that
possible?
Well, that's how it was done in previous revision. :)
I changed to a blacklist for consistency with block device's WCE feature
check after Jason's comment.
I'm not sure moving back to a whitelist brings much advantages when
compared to the effort of keeping it up to date. Just blacklisting
VIRTIO_NET_F_CTRL_VQ is enough in my opinion.
Thanks,
Maxime
Thanks,
Maxime
[0]:
https://github.com/oasis-tcs/virtio-spec/blob/5fc35a7efb903fc352da81a6d2be5c01810b68d3/device-types/net/description.tex#L129
Other than this,
Acked-by: Jason Wang
Thanks
+
struct vduse_virtqueue {
u16 index;
u16 num_max;
@@ -1680,6 +1690,9 @@ static bool features_is_valid(struct vduse_dev_config
*config)
if ((config->device_id == VIRTIO_ID_BLOCK) &&
(config->features & (1ULL <<
VIRTIO_BLK_F_CONFIG_WCE)))
return false;
+ else if ((config->device_id == VIRTIO_ID_NET) &&
+ (config->features & VDUSE_NET_INVALID_FEATURES_MASK))
+ return false;
return true;
}
--
2.43.0
Re: [PATCH v6 2/3] vduse: Temporarily fail if control queue features requested
On 1/5/24 03:45, Jason Wang wrote:
On Thu, Jan 4, 2024 at 11:38 PM Maxime Coquelin
wrote:
Virtio-net driver control queue implementation is not safe
when used with VDUSE. If the VDUSE application does not
reply to control queue messages, it currently ends up
hanging the kernel thread sending this command.
Some work is on-going to make the control queue
implementation robust with VDUSE. Until it is completed,
let's fail features check if any control-queue related
feature is requested.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0486ff672408..94f54ea2eb06 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include "iova_domain.h"
@@ -46,6 +47,15 @@
#define IRQ_UNBOUND -1
+#define VDUSE_NET_INVALID_FEATURES_MASK \
+ (BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |\
+BIT_ULL(VIRTIO_NET_F_CTRL_RX) | \
+BIT_ULL(VIRTIO_NET_F_CTRL_VLAN) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_ANNOUNCE) | \
+BIT_ULL(VIRTIO_NET_F_MQ) | \
+BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR) | \
+BIT_ULL(VIRTIO_NET_F_RSS))
We need to make this as well:
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
I missed it, and see others have been added in the Virtio spec
repository (BTW, I see this specific one is missing in the dependency
list [0], I will submit a patch).
I wonder if it is not just simpler to just check for
VIRTIO_NET_F_CTRL_VQ is requested. As we fail instead of masking out,
the VDUSE driver won't be the one violating the spec so it should be
good?
It will avoid having to update the mask if new features depending on it
are added (or forgetting to update it).
WDYT?
Thanks,
Maxime
[0]:
https://github.com/oasis-tcs/virtio-spec/blob/5fc35a7efb903fc352da81a6d2be5c01810b68d3/device-types/net/description.tex#L129
Other than this,
Acked-by: Jason Wang
Thanks
+
struct vduse_virtqueue {
u16 index;
u16 num_max;
@@ -1680,6 +1690,9 @@ static bool features_is_valid(struct vduse_dev_config
*config)
if ((config->device_id == VIRTIO_ID_BLOCK) &&
(config->features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE)))
return false;
+ else if ((config->device_id == VIRTIO_ID_NET) &&
+ (config->features & VDUSE_NET_INVALID_FEATURES_MASK))
+ return false;
return true;
}
--
2.43.0
[PATCH v6 3/3] vduse: enable Virtio-net device type
This patch adds Virtio-net device type to the supported
devices types.
Initialization fails if the device does not support
VIRTIO_F_VERSION_1 feature, in order to guarantee the
configuration space is read-only. It also fails with
-EPERM if the CAP_NET_ADMIN is missing.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 94f54ea2eb06..4057b34ff995 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -151,6 +151,7 @@ static struct workqueue_struct *vduse_irq_bound_wq;
static u32 allowed_device_id[] = {
VIRTIO_ID_BLOCK,
+ VIRTIO_ID_NET,
};
static inline struct vduse_dev *vdpa_to_vduse(struct vdpa_device *vdpa)
@@ -1694,6 +1695,10 @@ static bool features_is_valid(struct vduse_dev_config
*config)
(config->features & VDUSE_NET_INVALID_FEATURES_MASK))
return false;
+ if ((config->device_id == VIRTIO_ID_NET) &&
+ !(config->features & (1ULL << VIRTIO_F_VERSION_1)))
+ return false;
+
return true;
}
@@ -1801,6 +1806,10 @@ static int vduse_create_dev(struct vduse_dev_config
*config,
int ret;
struct vduse_dev *dev;
+ ret = -EPERM;
+ if ((config->device_id == VIRTIO_ID_NET) && !capable(CAP_NET_ADMIN))
+ goto err;
+
ret = -EEXIST;
if (vduse_find_dev(config->name))
goto err;
@@ -2044,6 +2053,7 @@ static const struct vdpa_mgmtdev_ops vdpa_dev_mgmtdev_ops
= {
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_BLOCK, VIRTIO_DEV_ANY_ID },
+ { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
};
--
2.43.0
[PATCH v6 2/3] vduse: Temporarily fail if control queue features requested
Virtio-net driver control queue implementation is not safe
when used with VDUSE. If the VDUSE application does not
reply to control queue messages, it currently ends up
hanging the kernel thread sending this command.
Some work is on-going to make the control queue
implementation robust with VDUSE. Until it is completed,
let's fail features check if any control-queue related
feature is requested.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0486ff672408..94f54ea2eb06 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include "iova_domain.h"
@@ -46,6 +47,15 @@
#define IRQ_UNBOUND -1
+#define VDUSE_NET_INVALID_FEATURES_MASK \
+ (BIT_ULL(VIRTIO_NET_F_CTRL_VQ) |\
+BIT_ULL(VIRTIO_NET_F_CTRL_RX) | \
+BIT_ULL(VIRTIO_NET_F_CTRL_VLAN) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_ANNOUNCE) | \
+BIT_ULL(VIRTIO_NET_F_MQ) | \
+BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR) | \
+BIT_ULL(VIRTIO_NET_F_RSS))
+
struct vduse_virtqueue {
u16 index;
u16 num_max;
@@ -1680,6 +1690,9 @@ static bool features_is_valid(struct vduse_dev_config
*config)
if ((config->device_id == VIRTIO_ID_BLOCK) &&
(config->features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE)))
return false;
+ else if ((config->device_id == VIRTIO_ID_NET) &&
+ (config->features & VDUSE_NET_INVALID_FEATURES_MASK))
+ return false;
return true;
}
--
2.43.0
[PATCH v6 1/3] vduse: validate block features only with block devices
This patch is preliminary work to enable network device
type support to VDUSE.
As VIRTIO_BLK_F_CONFIG_WCE shares the same value as
VIRTIO_NET_F_HOST_TSO4, we need to restrict its check
to Virtio-blk device type.
Acked-by: Jason Wang
Reviewed-by: Xie Yongji
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0ddd4b8abecb..0486ff672408 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1671,13 +1671,14 @@ static bool device_is_allowed(u32 device_id)
return false;
}
-static bool features_is_valid(u64 features)
+static bool features_is_valid(struct vduse_dev_config *config)
{
- if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
+ if (!(config->features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
return false;
/* Now we only support read-only configuration space */
- if (features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE))
+ if ((config->device_id == VIRTIO_ID_BLOCK) &&
+ (config->features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE)))
return false;
return true;
@@ -1704,7 +1705,7 @@ static bool vduse_validate_config(struct vduse_dev_config
*config)
if (!device_is_allowed(config->device_id))
return false;
- if (!features_is_valid(config->features))
+ if (!features_is_valid(config))
return false;
return true;
--
2.43.0
[PATCH v6 0/3] vduse: add support for networking devices
This small series enables virtio-net device type in VDUSE. With it, basic operation have been tested, both with virtio-vdpa and vhost-vdpa using DPDK Vhost library series adding VDUSE support using split rings layout (merged in DPDK v23.07-rc1). Control queue support (and so multiqueue) has also been tested, but requires a Kernel series from Jason Wang relaxing control queue polling [1] to function reliably, so while Jason rework is done, a patch is added to disable CVQ and features that depend on it (tested also with DPDK v23.07-rc1). In this v5, LSM hooks introduced in previous revision are unified into a single hook that covers below operations: - VDUSE_CREATE_DEV ioctl on VDUSE control file, - VDUSE_DESTROY_DEV ioctl on VDUSE control file, - open() on VDUSE device file. In combination with the operations permission, a device type permission has to be associated: - block: Virtio block device type, - net: Virtio networking device type. changes in v6! == - Remove SELinux support from the series, will be handled in a dedicated one. - Require CAP_NET_ADMIN for Net devices creation (Jason). - Fail init if control queue features are requested for Net device type (Jason). - Rebased on latest master. Changes in v5: == - Move control queue disablement patch before Net devices enablement (Jason). - Unify operations LSM hooks into a single hook. - Rebase on latest master. Maxime Coquelin (3): vduse: validate block features only with block devices vduse: Temporarily fail if control queue features requested vduse: enable Virtio-net device type drivers/vdpa/vdpa_user/vduse_dev.c | 32 ++ 1 file changed, 28 insertions(+), 4 deletions(-) -- 2.43.0
Re: [PATCH v5 4/4] vduse: Add LSM hook to check Virtio device type
On 12/18/23 18:33, Stephen Smalley wrote: On Mon, Dec 18, 2023 at 12:21 PM Stephen Smalley wrote: On Tue, Dec 12, 2023 at 8:17 AM Maxime Coquelin wrote: This patch introduces a LSM hook for devices creation, destruction (ioctl()) and opening (open()) operations, checking the application is allowed to perform these operations for the Virtio device type. Can you explain why the existing LSM hooks and SELinux implementation are not sufficient? We already control the ability to open device nodes via selinux_inode_permission() and selinux_file_open(), and can support fine-grained per-cmd ioctl checking via selinux_file_ioctl(). And it should already be possible to label these nodes distinctly through existing mechanisms (file_contexts if udev-created/labeled, genfs_contexts if kernel-created). What exactly can't you do today that this hook enables? (added Ondrej to the distribution; IMHO we should swap him into MAINTAINERS in place of Eric Paris since Eric has long-since moved on from SELinux and Ondrej serves in that capacity these days) Other items to consider: - If vduse devices are created using anonymous inodes, then SELinux grew a general facility for labeling and controlling the creation of those via selinux_inode_init_security_anon(). - You can encode information about the device into its SELinux type that then allows you to distinguish things like net vs block based on the device's SELinux security context rather than encoding that in the permission bits. Got it, that seems indeed more appropriate than using persmission bits for the device type. - If you truly need new LSM hooks (which you need to prove first), then you should pass some usable information about the object in question to allow SELinux to find a security context for it. Like an inode associated with the device, for example. Ok. Thanks for the insights, I'll try and see if I can follow your recommendations in a dedicated series. Maxime
Re: [PATCH v5 2/4] vduse: Temporarily disable control queue features
On 12/18/23 03:50, Jason Wang wrote:
On Wed, Dec 13, 2023 at 7:23 PM Maxime Coquelin
wrote:
Hi Jason,
On 12/13/23 05:52, Jason Wang wrote:
On Tue, Dec 12, 2023 at 9:17 PM Maxime Coquelin
wrote:
Virtio-net driver control queue implementation is not safe
when used with VDUSE. If the VDUSE application does not
reply to control queue messages, it currently ends up
hanging the kernel thread sending this command.
Some work is on-going to make the control queue
implementation robust with VDUSE. Until it is completed,
let's disable control virtqueue and features that depend on
it.
Signed-off-by: Maxime Coquelin
I wonder if it's better to fail instead of a mask as a start.
I think it is better to use a mask and not fail, so that we can in the
future use a recent VDUSE application with an older kernel.
It may confuse the userspace unless userspace can do post check after
CREATE_DEV.
And for blk we fail when WCE is set in feature_is_valid():
static bool features_is_valid(u64 features)
{
if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
return false;
/* Now we only support read-only configuration space */
if (features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE))
return false;
return true;
}
Ok, consistency with other devices types is indeed better.
But should I fail if any of the feature advertised by the application is
not listed by the VDUSE driver, or just fail if control queue is being
advertised by the application?
Thanks,
Maxime
Thanks
Why would it be better to fail than negotiating?
Thanks,
Maxime
Re: [PATCH v5 2/4] vduse: Temporarily disable control queue features
Hi Jason,
On 12/13/23 05:52, Jason Wang wrote:
On Tue, Dec 12, 2023 at 9:17 PM Maxime Coquelin
wrote:
Virtio-net driver control queue implementation is not safe
when used with VDUSE. If the VDUSE application does not
reply to control queue messages, it currently ends up
hanging the kernel thread sending this command.
Some work is on-going to make the control queue
implementation robust with VDUSE. Until it is completed,
let's disable control virtqueue and features that depend on
it.
Signed-off-by: Maxime Coquelin
I wonder if it's better to fail instead of a mask as a start.
I think it is better to use a mask and not fail, so that we can in the
future use a recent VDUSE application with an older kernel.
Why would it be better to fail than negotiating?
Thanks,
Maxime
Thanks
---
drivers/vdpa/vdpa_user/vduse_dev.c | 37 ++
1 file changed, 37 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0486ff672408..fe4b5c8203fd 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include "iova_domain.h"
@@ -46,6 +47,30 @@
#define IRQ_UNBOUND -1
+#define VDUSE_NET_VALID_FEATURES_MASK \
+ (BIT_ULL(VIRTIO_NET_F_CSUM) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_CSUM) | \
+BIT_ULL(VIRTIO_NET_F_MTU) |\
+BIT_ULL(VIRTIO_NET_F_MAC) |\
+BIT_ULL(VIRTIO_NET_F_GUEST_TSO4) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_TSO6) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_ECN) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_UFO) | \
+BIT_ULL(VIRTIO_NET_F_HOST_TSO4) | \
+BIT_ULL(VIRTIO_NET_F_HOST_TSO6) | \
+BIT_ULL(VIRTIO_NET_F_HOST_ECN) | \
+BIT_ULL(VIRTIO_NET_F_HOST_UFO) | \
+BIT_ULL(VIRTIO_NET_F_MRG_RXBUF) | \
+BIT_ULL(VIRTIO_NET_F_STATUS) | \
+BIT_ULL(VIRTIO_NET_F_HOST_USO) | \
+BIT_ULL(VIRTIO_F_ANY_LAYOUT) | \
+BIT_ULL(VIRTIO_RING_F_INDIRECT_DESC) | \
+BIT_ULL(VIRTIO_RING_F_EVENT_IDX) | \
+BIT_ULL(VIRTIO_F_VERSION_1) | \
+BIT_ULL(VIRTIO_F_ACCESS_PLATFORM) | \
+BIT_ULL(VIRTIO_F_RING_PACKED) |\
+BIT_ULL(VIRTIO_F_IN_ORDER))
+
struct vduse_virtqueue {
u16 index;
u16 num_max;
@@ -1782,6 +1807,16 @@ static struct attribute *vduse_dev_attrs[] = {
ATTRIBUTE_GROUPS(vduse_dev);
+static void vduse_dev_features_filter(struct vduse_dev_config *config)
+{
+ /*
+* Temporarily filter out virtio-net's control virtqueue and features
+* that depend on it while CVQ is being made more robust for VDUSE.
+*/
+ if (config->device_id == VIRTIO_ID_NET)
+ config->features &= VDUSE_NET_VALID_FEATURES_MASK;
+}
+
static int vduse_create_dev(struct vduse_dev_config *config,
void *config_buf, u64 api_version)
{
@@ -1797,6 +1832,8 @@ static int vduse_create_dev(struct vduse_dev_config
*config,
if (!dev)
goto err;
+ vduse_dev_features_filter(config);
+
dev->api_version = api_version;
dev->device_features = config->features;
dev->device_id = config->device_id;
--
2.43.0
[PATCH v5 4/4] vduse: Add LSM hook to check Virtio device type
This patch introduces a LSM hook for devices creation,
destruction (ioctl()) and opening (open()) operations,
checking the application is allowed to perform these
operations for the Virtio device type.
Signed-off-by: Maxime Coquelin
---
MAINTAINERS | 1 +
drivers/vdpa/vdpa_user/vduse_dev.c | 13
include/linux/lsm_hook_defs.h | 2 ++
include/linux/security.h| 6 ++
include/linux/vduse.h | 14 +
security/security.c | 15 ++
security/selinux/hooks.c| 32 +
security/selinux/include/classmap.h | 2 ++
8 files changed, 85 insertions(+)
create mode 100644 include/linux/vduse.h
diff --git a/MAINTAINERS b/MAINTAINERS
index a0fb0df07b43..4e83b14358d2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23040,6 +23040,7 @@ F: drivers/net/virtio_net.c
F: drivers/vdpa/
F: drivers/virtio/
F: include/linux/vdpa.h
+F: include/linux/vduse.h
F: include/linux/virtio*.h
F: include/linux/vringh.h
F: include/uapi/linux/virtio_*.h
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index fa62825be378..59ab7eb62e20 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -8,6 +8,7 @@
*
*/
+#include "linux/security.h"
#include
#include
#include
@@ -30,6 +31,7 @@
#include
#include
#include
+#include
#include "iova_domain.h"
@@ -1442,6 +1444,10 @@ static int vduse_dev_open(struct inode *inode, struct
file *file)
if (dev->connected)
goto unlock;
+ ret = -EPERM;
+ if (security_vduse_perm_check(VDUSE_PERM_OPEN, dev->device_id))
+ goto unlock;
+
ret = 0;
dev->connected = true;
file->private_data = dev;
@@ -1664,6 +1670,9 @@ static int vduse_destroy_dev(char *name)
if (!dev)
return -EINVAL;
+ if (security_vduse_perm_check(VDUSE_PERM_DESTROY, dev->device_id))
+ return -EPERM;
+
mutex_lock(&dev->lock);
if (dev->vdev || dev->connected) {
mutex_unlock(&dev->lock);
@@ -1828,6 +1837,10 @@ static int vduse_create_dev(struct vduse_dev_config
*config,
int ret;
struct vduse_dev *dev;
+ ret = -EPERM;
+ if (security_vduse_perm_check(VDUSE_PERM_CREATE, config->device_id))
+ goto err;
+
ret = -EEXIST;
if (vduse_find_dev(config->name))
goto err;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index ff217a5ce552..3930ab2ae974 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -419,3 +419,5 @@ LSM_HOOK(int, 0, uring_override_creds, const struct cred
*new)
LSM_HOOK(int, 0, uring_sqpoll, void)
LSM_HOOK(int, 0, uring_cmd, struct io_uring_cmd *ioucmd)
#endif /* CONFIG_IO_URING */
+
+LSM_HOOK(int, 0, vduse_perm_check, enum vduse_op_perm op_perm, u32 device_id)
diff --git a/include/linux/security.h b/include/linux/security.h
index 1d1df326c881..2a2054172394 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -32,6 +32,7 @@
#include
#include
#include
+#include
struct linux_binprm;
struct cred;
@@ -484,6 +485,7 @@ int security_inode_notifysecctx(struct inode *inode, void
*ctx, u32 ctxlen);
int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
int security_locked_down(enum lockdown_reason what);
+int security_vduse_perm_check(enum vduse_op_perm op_perm, u32 device_id);
#else /* CONFIG_SECURITY */
static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
@@ -1395,6 +1397,10 @@ static inline int security_locked_down(enum
lockdown_reason what)
{
return 0;
}
+static inline int security_vduse_perm_check(enum vduse_op_perm op_perm, u32
device_id)
+{
+ return 0;
+}
#endif /* CONFIG_SECURITY */
#if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
diff --git a/include/linux/vduse.h b/include/linux/vduse.h
new file mode 100644
index ..7a20dcc43997
--- /dev/null
+++ b/include/linux/vduse.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_VDUSE_H
+#define _LINUX_VDUSE_H
+
+/*
+ * The permission required for a VDUSE device operation.
+ */
+enum vduse_op_perm {
+ VDUSE_PERM_CREATE,
+ VDUSE_PERM_DESTROY,
+ VDUSE_PERM_OPEN,
+};
+
+#endif /* _LINUX_VDUSE_H */
diff --git a/security/security.c b/security/security.c
index dcb3e7014f9b..150abf85f97d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -5337,3 +5337,18 @@ int security_uring_cmd(struct io_uring_cmd *ioucmd)
return call_int_hook(uring_cmd, 0, ioucmd);
}
#endif /* CONFIG_IO_URING */
+
+/**
+ * security_vduse_perm_check() -
[PATCH v5 2/4] vduse: Temporarily disable control queue features
Virtio-net driver control queue implementation is not safe
when used with VDUSE. If the VDUSE application does not
reply to control queue messages, it currently ends up
hanging the kernel thread sending this command.
Some work is on-going to make the control queue
implementation robust with VDUSE. Until it is completed,
let's disable control virtqueue and features that depend on
it.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 37 ++
1 file changed, 37 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0486ff672408..fe4b5c8203fd 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include "iova_domain.h"
@@ -46,6 +47,30 @@
#define IRQ_UNBOUND -1
+#define VDUSE_NET_VALID_FEATURES_MASK \
+ (BIT_ULL(VIRTIO_NET_F_CSUM) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_CSUM) | \
+BIT_ULL(VIRTIO_NET_F_MTU) |\
+BIT_ULL(VIRTIO_NET_F_MAC) |\
+BIT_ULL(VIRTIO_NET_F_GUEST_TSO4) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_TSO6) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_ECN) | \
+BIT_ULL(VIRTIO_NET_F_GUEST_UFO) | \
+BIT_ULL(VIRTIO_NET_F_HOST_TSO4) | \
+BIT_ULL(VIRTIO_NET_F_HOST_TSO6) | \
+BIT_ULL(VIRTIO_NET_F_HOST_ECN) | \
+BIT_ULL(VIRTIO_NET_F_HOST_UFO) | \
+BIT_ULL(VIRTIO_NET_F_MRG_RXBUF) | \
+BIT_ULL(VIRTIO_NET_F_STATUS) | \
+BIT_ULL(VIRTIO_NET_F_HOST_USO) | \
+BIT_ULL(VIRTIO_F_ANY_LAYOUT) | \
+BIT_ULL(VIRTIO_RING_F_INDIRECT_DESC) | \
+BIT_ULL(VIRTIO_RING_F_EVENT_IDX) | \
+BIT_ULL(VIRTIO_F_VERSION_1) | \
+BIT_ULL(VIRTIO_F_ACCESS_PLATFORM) | \
+BIT_ULL(VIRTIO_F_RING_PACKED) |\
+BIT_ULL(VIRTIO_F_IN_ORDER))
+
struct vduse_virtqueue {
u16 index;
u16 num_max;
@@ -1782,6 +1807,16 @@ static struct attribute *vduse_dev_attrs[] = {
ATTRIBUTE_GROUPS(vduse_dev);
+static void vduse_dev_features_filter(struct vduse_dev_config *config)
+{
+ /*
+* Temporarily filter out virtio-net's control virtqueue and features
+* that depend on it while CVQ is being made more robust for VDUSE.
+*/
+ if (config->device_id == VIRTIO_ID_NET)
+ config->features &= VDUSE_NET_VALID_FEATURES_MASK;
+}
+
static int vduse_create_dev(struct vduse_dev_config *config,
void *config_buf, u64 api_version)
{
@@ -1797,6 +1832,8 @@ static int vduse_create_dev(struct vduse_dev_config
*config,
if (!dev)
goto err;
+ vduse_dev_features_filter(config);
+
dev->api_version = api_version;
dev->device_features = config->features;
dev->device_id = config->device_id;
--
2.43.0
[PATCH v5 3/4] vduse: enable Virtio-net device type
This patch adds Virtio-net device type to the supported
devices types. Initialization fails if the device does
not support VIRTIO_F_VERSION_1 feature, in order to
guarantee the configuration space is read-only.
Acked-by: Jason Wang
Reviewed-by: Xie Yongji
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index fe4b5c8203fd..fa62825be378 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -166,6 +166,7 @@ static struct workqueue_struct *vduse_irq_bound_wq;
static u32 allowed_device_id[] = {
VIRTIO_ID_BLOCK,
+ VIRTIO_ID_NET,
};
static inline struct vduse_dev *vdpa_to_vduse(struct vdpa_device *vdpa)
@@ -1706,6 +1707,10 @@ static bool features_is_valid(struct vduse_dev_config
*config)
(config->features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE)))
return false;
+ if ((config->device_id == VIRTIO_ID_NET) &&
+ !(config->features & (1ULL << VIRTIO_F_VERSION_1)))
+ return false;
+
return true;
}
@@ -2068,6 +2073,7 @@ static const struct vdpa_mgmtdev_ops vdpa_dev_mgmtdev_ops
= {
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_BLOCK, VIRTIO_DEV_ANY_ID },
+ { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
};
--
2.43.0
[PATCH v5 1/4] vduse: validate block features only with block devices
This patch is preliminary work to enable network device
type support to VDUSE.
As VIRTIO_BLK_F_CONFIG_WCE shares the same value as
VIRTIO_NET_F_HOST_TSO4, we need to restrict its check
to Virtio-blk device type.
Acked-by: Jason Wang
Reviewed-by: Xie Yongji
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
index 0ddd4b8abecb..0486ff672408 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1671,13 +1671,14 @@ static bool device_is_allowed(u32 device_id)
return false;
}
-static bool features_is_valid(u64 features)
+static bool features_is_valid(struct vduse_dev_config *config)
{
- if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
+ if (!(config->features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
return false;
/* Now we only support read-only configuration space */
- if (features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE))
+ if ((config->device_id == VIRTIO_ID_BLOCK) &&
+ (config->features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE)))
return false;
return true;
@@ -1704,7 +1705,7 @@ static bool vduse_validate_config(struct vduse_dev_config
*config)
if (!device_is_allowed(config->device_id))
return false;
- if (!features_is_valid(config->features))
+ if (!features_is_valid(config))
return false;
return true;
--
2.43.0
[PATCH v5 0/4] vduse: add support for networking devices
This small series enables virtio-net device type in VDUSE. With it, basic operation have been tested, both with virtio-vdpa and vhost-vdpa using DPDK Vhost library series adding VDUSE support using split rings layout (merged in DPDK v23.07-rc1). Control queue support (and so multiqueue) has also been tested, but requires a Kernel series from Jason Wang relaxing control queue polling [1] to function reliably, so while Jason rework is done, a patch is added to disable CVQ and features that depend on it (tested also with DPDK v23.07-rc1). In this v5, LSM hooks introduced in previous revision are unified into a single hook that covers below operations: - VDUSE_CREATE_DEV ioctl on VDUSE control file, - VDUSE_DESTROY_DEV ioctl on VDUSE control file, - open() on VDUSE device file. In combination with the operations permission, a device type permission has to be associated: - block: Virtio block device type, - net: Virtio networking device type. Changes in v5: == - Move control queue disablement patch before Net devices enablement (Jason). - Unify operations LSM hooks into a single hook. - Rebase on latest master. Maxime Coquelin (4): vduse: validate block features only with block devices vduse: Temporarily disable control queue features vduse: enable Virtio-net device type vduse: Add LSM hook to check Virtio device type MAINTAINERS | 1 + drivers/vdpa/vdpa_user/vduse_dev.c | 65 +++-- include/linux/lsm_hook_defs.h | 2 + include/linux/security.h| 6 +++ include/linux/vduse.h | 14 +++ security/security.c | 15 +++ security/selinux/hooks.c| 32 ++ security/selinux/include/classmap.h | 2 + 8 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 include/linux/vduse.h -- 2.43.0
Re: [PATCH v4 4/4] vduse: Add LSM hooks to check Virtio device type
On 12/8/23 13:26, Michael S. Tsirkin wrote:
On Fri, Dec 08, 2023 at 01:23:00PM +0100, Maxime Coquelin wrote:
On 12/8/23 12:05, Michael S. Tsirkin wrote:
On Fri, Dec 08, 2023 at 12:01:15PM +0100, Maxime Coquelin wrote:
Hello Paul,
On 11/8/23 03:31, Paul Moore wrote:
On Oct 20, 2023 "Michael S. Tsirkin" wrote:
This patch introduces LSM hooks for devices creation,
destruction and opening operations, checking the
application is allowed to perform these operations for
the Virtio device type.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 12 +++
include/linux/lsm_hook_defs.h | 4 +++
include/linux/security.h| 15
security/security.c | 42 ++
security/selinux/hooks.c| 55 +
security/selinux/include/classmap.h | 2 ++
6 files changed, 130 insertions(+)
My apologies for the late reply, I've been trying to work my way through
the review backlog but it has been taking longer than expected; comments
below ...
No worries, I have also been busy these days.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2aa0e219d721..65d9262a37f7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -21,6 +21,7 @@
* Copyright (C) 2016 Mellanox Technologies
*/
+#include "av_permissions.h"
#include
#include
#include
@@ -92,6 +93,7 @@
#include
#include
#include
+#include
#include "avc.h"
#include "objsec.h"
@@ -6950,6 +6952,56 @@ static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
}
#endif /* CONFIG_IO_URING */
+static int vduse_check_device_type(u32 sid, u32 device_id)
+{
+ u32 requested;
+
+ if (device_id == VIRTIO_ID_NET)
+ requested = VDUSE__NET;
+ else if (device_id == VIRTIO_ID_BLOCK)
+ requested = VDUSE__BLOCK;
+ else
+ return -EINVAL;
+
+ return avc_has_perm(sid, sid, SECCLASS_VDUSE, requested, NULL);
+}
+
+static int selinux_vduse_dev_create(u32 device_id)
+{
+ u32 sid = current_sid();
+ int ret;
+
+ ret = avc_has_perm(sid, sid, SECCLASS_VDUSE, VDUSE__DEVCREATE, NULL);
+ if (ret)
+ return ret;
+
+ return vduse_check_device_type(sid, device_id);
+}
I see there has been some discussion about the need for a dedicated
create hook as opposed to using the existing ioctl controls. I think
one important point that has been missing from the discussion is the
idea of labeling the newly created device. Unfortunately prior to a
few minutes ago I hadn't ever looked at VDUSE so please correct me if
I get some things wrong :)
From what I can see userspace creates a new VDUSE device with
ioctl(VDUSE_CREATE_DEV), which trigger the creation of a new
/dev/vduse/XXX device which will be labeled according to the udev
and SELinux configuration, likely with a generic udev label. My
question is if we want to be able to uniquely label each VDUSE
device based on the process that initiates the device creation
with the call to ioctl()? If that is the case, we would need a
create hook not only to control the creation of the device, but to
record the triggering process' label in the new device; this label
would then be used in subsequent VDUSE open and destroy operations.
The normal device file I/O operations would still be subject to the
standard SELinux file I/O permissions using the device file label
assigned by systemd/udev when the device was created.
I don't think we need a unique label for VDUSE devices, but maybe
Michael thinks otherwise?
I don't know.
All this is consumed by libvirt, you need to ask these guys.
I think it is not consumed by libvirt, at least not in the usecases I
have in mind. For networking devices, it will be consumed by OVS.
Maxime
OK, ovs then :)
Adding Aaron, our go-to person for SELinux-related topics for OVS, but I
think we don't need to do relabeling for VDUSE chardevs.
Aaron, do you confirm?
Maxime
Re: [PATCH v4 4/4] vduse: Add LSM hooks to check Virtio device type
On 12/8/23 12:05, Michael S. Tsirkin wrote:
On Fri, Dec 08, 2023 at 12:01:15PM +0100, Maxime Coquelin wrote:
Hello Paul,
On 11/8/23 03:31, Paul Moore wrote:
On Oct 20, 2023 "Michael S. Tsirkin" wrote:
This patch introduces LSM hooks for devices creation,
destruction and opening operations, checking the
application is allowed to perform these operations for
the Virtio device type.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 12 +++
include/linux/lsm_hook_defs.h | 4 +++
include/linux/security.h| 15
security/security.c | 42 ++
security/selinux/hooks.c| 55 +
security/selinux/include/classmap.h | 2 ++
6 files changed, 130 insertions(+)
My apologies for the late reply, I've been trying to work my way through
the review backlog but it has been taking longer than expected; comments
below ...
No worries, I have also been busy these days.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2aa0e219d721..65d9262a37f7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -21,6 +21,7 @@
* Copyright (C) 2016 Mellanox Technologies
*/
+#include "av_permissions.h"
#include
#include
#include
@@ -92,6 +93,7 @@
#include
#include
#include
+#include
#include "avc.h"
#include "objsec.h"
@@ -6950,6 +6952,56 @@ static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
}
#endif /* CONFIG_IO_URING */
+static int vduse_check_device_type(u32 sid, u32 device_id)
+{
+ u32 requested;
+
+ if (device_id == VIRTIO_ID_NET)
+ requested = VDUSE__NET;
+ else if (device_id == VIRTIO_ID_BLOCK)
+ requested = VDUSE__BLOCK;
+ else
+ return -EINVAL;
+
+ return avc_has_perm(sid, sid, SECCLASS_VDUSE, requested, NULL);
+}
+
+static int selinux_vduse_dev_create(u32 device_id)
+{
+ u32 sid = current_sid();
+ int ret;
+
+ ret = avc_has_perm(sid, sid, SECCLASS_VDUSE, VDUSE__DEVCREATE, NULL);
+ if (ret)
+ return ret;
+
+ return vduse_check_device_type(sid, device_id);
+}
I see there has been some discussion about the need for a dedicated
create hook as opposed to using the existing ioctl controls. I think
one important point that has been missing from the discussion is the
idea of labeling the newly created device. Unfortunately prior to a
few minutes ago I hadn't ever looked at VDUSE so please correct me if
I get some things wrong :)
From what I can see userspace creates a new VDUSE device with
ioctl(VDUSE_CREATE_DEV), which trigger the creation of a new
/dev/vduse/XXX device which will be labeled according to the udev
and SELinux configuration, likely with a generic udev label. My
question is if we want to be able to uniquely label each VDUSE
device based on the process that initiates the device creation
with the call to ioctl()? If that is the case, we would need a
create hook not only to control the creation of the device, but to
record the triggering process' label in the new device; this label
would then be used in subsequent VDUSE open and destroy operations.
The normal device file I/O operations would still be subject to the
standard SELinux file I/O permissions using the device file label
assigned by systemd/udev when the device was created.
I don't think we need a unique label for VDUSE devices, but maybe
Michael thinks otherwise?
I don't know.
All this is consumed by libvirt, you need to ask these guys.
I think it is not consumed by libvirt, at least not in the usecases I
have in mind. For networking devices, it will be consumed by OVS.
Maxime
Re: [PATCH v4 4/4] vduse: Add LSM hooks to check Virtio device type
Hello Paul,
On 11/8/23 03:31, Paul Moore wrote:
On Oct 20, 2023 "Michael S. Tsirkin" wrote:
This patch introduces LSM hooks for devices creation,
destruction and opening operations, checking the
application is allowed to perform these operations for
the Virtio device type.
Signed-off-by: Maxime Coquelin
---
drivers/vdpa/vdpa_user/vduse_dev.c | 12 +++
include/linux/lsm_hook_defs.h | 4 +++
include/linux/security.h| 15
security/security.c | 42 ++
security/selinux/hooks.c| 55 +
security/selinux/include/classmap.h | 2 ++
6 files changed, 130 insertions(+)
My apologies for the late reply, I've been trying to work my way through
the review backlog but it has been taking longer than expected; comments
below ...
No worries, I have also been busy these days.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2aa0e219d721..65d9262a37f7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -21,6 +21,7 @@
* Copyright (C) 2016 Mellanox Technologies
*/
+#include "av_permissions.h"
#include
#include
#include
@@ -92,6 +93,7 @@
#include
#include
#include
+#include
#include "avc.h"
#include "objsec.h"
@@ -6950,6 +6952,56 @@ static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
}
#endif /* CONFIG_IO_URING */
+static int vduse_check_device_type(u32 sid, u32 device_id)
+{
+ u32 requested;
+
+ if (device_id == VIRTIO_ID_NET)
+ requested = VDUSE__NET;
+ else if (device_id == VIRTIO_ID_BLOCK)
+ requested = VDUSE__BLOCK;
+ else
+ return -EINVAL;
+
+ return avc_has_perm(sid, sid, SECCLASS_VDUSE, requested, NULL);
+}
+
+static int selinux_vduse_dev_create(u32 device_id)
+{
+ u32 sid = current_sid();
+ int ret;
+
+ ret = avc_has_perm(sid, sid, SECCLASS_VDUSE, VDUSE__DEVCREATE, NULL);
+ if (ret)
+ return ret;
+
+ return vduse_check_device_type(sid, device_id);
+}
I see there has been some discussion about the need for a dedicated
create hook as opposed to using the existing ioctl controls. I think
one important point that has been missing from the discussion is the
idea of labeling the newly created device. Unfortunately prior to a
few minutes ago I hadn't ever looked at VDUSE so please correct me if
I get some things wrong :)
From what I can see userspace creates a new VDUSE device with
ioctl(VDUSE_CREATE_DEV), which trigger the creation of a new
/dev/vduse/XXX device which will be labeled according to the udev
and SELinux configuration, likely with a generic udev label. My
question is if we want to be able to uniquely label each VDUSE
device based on the process that initiates the device creation
with the call to ioctl()? If that is the case, we would need a
create hook not only to control the creation of the device, but to
record the triggering process' label in the new device; this label
would then be used in subsequent VDUSE open and destroy operations.
The normal device file I/O operations would still be subject to the
standard SELinux file I/O permissions using the device file label
assigned by systemd/udev when the device was created.
I don't think we need a unique label for VDUSE devices, but maybe
Michael thinks otherwise?
+static int selinux_vduse_dev_destroy(u32 device_id)
+{
+ u32 sid = current_sid();
+ int ret;
+
+ ret = avc_has_perm(sid, sid, SECCLASS_VDUSE, VDUSE__DEVDESTROY, NULL);
+ if (ret)
+ return ret;
+
+ return vduse_check_device_type(sid, device_id);
+}
+
+static int selinux_vduse_dev_open(u32 device_id)
+{
+ u32 sid = current_sid();
+ int ret;
+
+ ret = avc_has_perm(sid, sid, SECCLASS_VDUSE, VDUSE__DEVOPEN, NULL);
+ if (ret)
+ return ret;
+
+ return vduse_check_device_type(sid, device_id);
+}
+
/*
* IMPORTANT NOTE: When adding new hooks, please be careful to keep this
order:
* 1. any hooks that don't belong to (2.) or (3.) below,
@@ -7243,6 +7295,9 @@ static struct security_hook_list selinux_hooks[]
__ro_after_init = {
#ifdef CONFIG_PERF_EVENTS
LSM_HOOK_INIT(perf_event_alloc, selinux_perf_event_alloc),
#endif
+ LSM_HOOK_INIT(vduse_dev_create, selinux_vduse_dev_create),
+ LSM_HOOK_INIT(vduse_dev_destroy, selinux_vduse_dev_destroy),
+ LSM_HOOK_INIT(vduse_dev_open, selinux_vduse_dev_open),
};
static __init int selinux_init(void)
diff --git a/security/selinux/include/classmap.h
b/security/selinux/include/classmap.h
index a3c380775d41..d3dc37fb03d4 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -256,6 +256,8 @@ const struct security_class_mapping secclass_map[] = {
{ "override_creds", "
Re: [PATCH 1/2] vdpa: ifcvf: return err when fail to request config irq
Hi,
On 7/23/20 11:12 AM, Jason Wang wrote:
> We ignore the err of requesting config interrupt, fix this.
>
> Fixes: e7991f376a4d ("ifcvf: implement config interrupt in IFCVF")
> Cc: Zhu Lingshan
> Signed-off-by: Jason Wang
> ---
> drivers/vdpa/ifcvf/ifcvf_main.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index f5a60c14b979..ae7110955a44 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -76,6 +76,10 @@ static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
> ret = devm_request_irq(&pdev->dev, irq,
> ifcvf_config_changed, 0,
> vf->config_msix_name, vf);
> + if (ret) {
> + IFCVF_ERR(pdev, "Failed to request config irq\n");
> + return ret;
> + }
>
> for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
> snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",
>
This series fixes below Kernel BUG I faced while doing some experiments:
[ 1398.695362] kernel BUG at drivers/pci/msi.c:375!
[ 1398.700561] invalid opcode: [#1] SMP PTI
[ 1398.705423] CPU: 0 PID: 25110 Comm: dpdk-testpmd Not tainted
5.8.0-amorenoz-vdpa+ #2
[ 1398.714063] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS
2.4.3 01/17/2017
[ 1398.722415] RIP: 0010:free_msi_irqs+0x189/0x1c0
[ 1398.727470] Code: 14 85 c0 0f 84 cc fe ff ff 31 ed eb 0f 83 c5 01 39
6b 14 0f 86 bc fe ff ff 8b 7b 10 01 ef e8 7e 94 b9 ff 48 83 78 70 00d
[ 1398.748426] RSP: 0018:b48ac5dd3db8 EFLAGS: 00010286
[ 1398.754257] RAX: 9ab298b85400 RBX: 9ab285d97100 RCX:
[ 1398.762219] RDX: RSI: 0073 RDI:
ac65e8a0
[ 1398.770182] RBP: R08: 9ab298b85400 R09:
9ab74a8c3d98
[ 1398.778145] R10: 9ab74a8c3f58 R11: R12:
9ab719fd82e0
[ 1398.786107] R13: 9ab719fd8000 R14: 9ab719fd8000 R15:
9ab719fd80b0
[ 1398.794069] FS: 7efc5dea9000() GS:9ab75fc0()
knlGS:
[ 1398.803099] CS: 0010 DS: ES: CR0: 80050033
[ 1398.809508] CR2: 00c79ff8 CR3: 00038283e005 CR4:
003606f0
[ 1398.817471] DR0: DR1: DR2:
[ 1398.825434] DR3: DR6: fffe0ff0 DR7:
0400
[ 1398.833394] Call Trace:
[ 1398.836127] pci_disable_msix+0xf7/0x120
[ 1398.840504] pci_free_irq_vectors+0xe/0x20
[ 1398.845074] ifcvf_vdpa_set_status+0xda/0x301
[ 1398.849938] vhost_vdpa_unlocked_ioctl+0x61d/0x790
[ 1398.855277] ? vhost_vdpa_process_iotlb_msg+0x2f0/0x2f0
[ 1398.861109] ksys_ioctl+0x87/0xc0
[ 1398.864808] __x64_sys_ioctl+0x16/0x20
[ 1398.868992] do_syscall_64+0x52/0x90
[ 1398.872982] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 1398.878610] RIP: 0033:0x7efc5df9ff9b
[ 1398.882598] Code: 0f 1e fa 48 8b 05 ed ce 0c 00 64 c7 00 26 00 00 00
48 c7 c0 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa b8 10 00 00 008
[ 1398.903551] RSP: 002b:7ffd0948e378 EFLAGS: 0283 ORIG_RAX:
0010
[ 1398.911998] RAX: ffda RBX: RCX:
7efc5df9ff9b
[ 1398.919960] RDX: 7ffd0948e3d4 RSI: 4001af72 RDI:
002c
[ 1398.927921] RBP: 7ffd0948e3c0 R08: 02651bf8 R09:
[ 1398.935883] R10: 7ffd0948e417 R11: 0283 R12:
00408950
[ 1398.943845] R13: 7ffd0948e6a0 R14: R15:
[ 1398.951809] Modules linked in: vxlan ip6_udp_tunnel udp_tunnel
ip_vs_sh ip_vs_wrr ip_vs_rr ip_vs xt_comment xt_mark nf_tables xt_nat vetht
[ 1398.951847] ghash_clmulni_intel iTCO_vendor_support mlx5_core dcdbas
rapl intel_cstate intel_uncore ipmi_ssif pcspkr mxm_wmi mlxfw virtii
Tested-by: Maxime Coquelin
Thanks,
Maxime
Re: [PATCH] watchdog: Respect watchdog cpumask on CPU hotplug
On 3/27/19 8:10 PM, Thomas Gleixner wrote: On Wed, 27 Mar 2019, Oleg Nesterov wrote: On 03/26, Thomas Gleixner wrote: The rework of the watchdog core to use cpu_stop_work broke the watchdog cpumask on CPU hotplug. The watchdog_enable/disable() functions are now called unconditionally from the hotplug callback, i.e. even on CPUs which are not in the watchdog cpumask. Only invoke them when the plugged CPU is in the watchdog cpumask. IIUC without this fix an NMI watchdog can too be enabled at boot time even if the initial watchdog_cpumask = housekeeping_cpumask(HK_FLAG_TIMER) doesn't include the plugged CPU. Yes. And after that writing 0 to /proc/sys/kernel/nmi_watchdog clears NMI_WATCHDOG_ENABLED but this can't disable NMI watchdog's outside of watchdog_allowed_mask. Correct So may be this can explain the problem reported by Maxime ? See https://lore.kernel.org/lkml/b99c5a25-a5fe-18dd-2f1d-bdd6834f0...@redhat.com/ That looks so. I had a trial with your patch, and I can confirm it fixes my issue: Tested-by: Maxime Coquelin Thanks, Maxime Thanks, tglx
[Regression]: NMI watchdog regression from v4.19 onwards
Hi Peter, Oleg, NMI watchdog fires systematically on my machine with recent Kernels, whereas the NMI watch is supposed to be disabled: # cat /proc/sys/kernel/watchdog 0 # cat /proc/sys/kernel/nmi_watchdog 0 # [ 53.765648] NMI watchdog: Watchdog detected hard LOCKUP on cpu 7 [ 53.765648] Modules linked in: nft_chain_route_ipv4 xt_CHECKSUM nft_chain_nat_ipv4 ipt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_counter nft_cd [ 53.765661] CPU: 7 PID: 0 Comm: swapper/7 Not tainted 5.0.0 #22 [ 53.765661] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.4.3 01/17/2017 [ 53.765661] RIP: 0010:poll_idle+0xa0/0x102 [ 53.765662] Code: 54 48 69 db e8 03 00 00 eb 1b f3 90 83 e8 01 75 19 65 8b 3d e2 a4 5b 6e e8 fd 9f 8a ff 48 29 e8 48 39 d8 77 60 b8 c9 00 00 00 <65> 48 8b5 [ 53.765662] RSP: :a77ac01f3e70 EFLAGS: 0206 [ 53.765663] RAX: 000a RBX: 07d0 RCX: 001f [ 53.765663] RDX: RSI: 28000347 RDI: f386de8dd002 [ 53.765663] RBP: 000c84ad17fc R08: 0004 R09: 00021540 [ 53.765664] R10: 28124ceb5144 R11: 0010 R12: 9905dfbea648 [ 53.765664] R13: R14: R15: [ 53.765664] FS: () GS:9905dfbc() knlGS: [ 53.765665] CS: 0010 DS: ES: CR0: 80050033 [ 53.765665] CR2: CR3: 00037f60e001 CR4: 003606e0 [ 53.765665] DR0: DR1: DR2: [ 53.765665] DR3: DR6: fffe0ff0 DR7: 0400 [ 53.765666] Call Trace: [ 53.765666] cpuidle_enter_state+0x73/0x440 [ 53.765666] do_idle+0x1f1/0x230 [ 53.765666] cpu_startup_entry+0x19/0x20 [ 53.765667] start_secondary+0x195/0x1e0 [ 53.765667] secondary_startup_64+0xb6/0xc0 [ 53.765667] Kernel panic - not syncing: Hard LOCKUP [ 53.765667] CPU: 7 PID: 0 Comm: swapper/7 Not tainted 5.0.0 #22 [ 53.765668] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.4.3 01/17/2017 [ 53.765668] Call Trace: [ 53.765668] [ 53.765668] dump_stack+0x46/0x60 [ 53.765669] panic+0xfb/0x29b [ 53.765669] ? startup_64+0x1/0x30 [ 53.765669] ? startup_64+0x30/0x30 [ 53.765669] nmi_panic.cold.9+0xc/0xc [ 53.765670] watchdog_overflow_callback.cold.7+0x5c/0x70 [ 53.765670] __perf_event_overflow+0x52/0xe0 [ 53.765670] handle_pmi_common+0x1b3/0x210 [ 53.765670] ? set_pte_vaddr_p4d+0x4a/0x60 [ 53.765671] ? ghes_copy_tofrom_phys+0xc0/0x170 [ 53.765671] intel_pmu_handle_irq+0xc9/0x1c0 [ 53.765671] perf_event_nmi_handler+0x2d/0x50 [ 53.765671] nmi_handle+0x63/0x110 [ 53.765672] default_do_nmi+0x4e/0x100 [ 53.765672] do_nmi+0x100/0x160 [ 53.765672] end_repeat_nmi+0x16/0x50 [ 53.765672] RIP: 0010:poll_idle+0xa0/0x102 [ 53.765673] Code: 54 48 69 db e8 03 00 00 eb 1b f3 90 83 e8 01 75 19 65 8b 3d e2 a4 5b 6e e8 fd 9f 8a ff 48 29 e8 48 39 d8 77 60 b8 c9 00 00 00 <65> 48 8b5 [ 53.765673] RSP: :a77ac01f3e70 EFLAGS: 0206 [ 53.765674] RAX: 000a RBX: 07d0 RCX: 001f [ 53.765674] RDX: RSI: 28000347 RDI: f386de8dd002 [ 53.765674] RBP: 000c84ad17fc R08: 0004 R09: 00021540 [ 53.765675] R10: 28124ceb5144 R11: 0010 R12: 9905dfbea648 [ 53.765675] R13: R14: R15: [ 53.765675] ? poll_idle+0xa0/0x102 [ 53.765675] ? poll_idle+0xa0/0x102 [ 53.765676] [ 53.765676] cpuidle_enter_state+0x73/0x440 [ 53.765676] do_idle+0x1f1/0x230 [ 53.765676] cpu_startup_entry+0x19/0x20 [ 53.765677] start_secondary+0x195/0x1e0 [ 53.765677] secondary_startup_64+0xb6/0xc0 [ 54.804636] NMI watchdog: Watchdog detected hard LOCKUP on cpu 6 [ 54.804637] Modules linked in: nft_chain_route_ipv4 xt_CHECKSUM nft_chain_nat_ipv4 ipt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_counter nft_cd [ 54.804649] CPU: 6 PID: 0 Comm: swapper/6 Not tainted 5.0.0 #22 [ 54.804649] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.4.3 01/17/2017 [ 54.804649] RIP: 0010:poll_idle+0xa0/0x102 [ 54.804650] Code: 54 48 69 db e8 03 00 00 eb 1b f3 90 83 e8 01 75 19 65 8b 3d e2 a4 5b 6e e8 fd 9f 8a ff 48 29 e8 48 39 d8 77 60 b8 c9 00 00 00 <65> 48 8b5 [ 54.804650] RSP: :a77ac01ebe70 EFLAGS: 0202 [ 54.804650] RAX: 0046 RBX: 07d0 RCX: 001f [ 54.804651] RDX: RSI: 28000347 RDI: f386de8dd002 [ 54.804651] RBP: 000c84aebd9f R08: 0002 R09: 00021540 [ 54.804651] R10: 28124cf09684 R11: 0032 R12: 9905dfbaa648 [ 54.804652] R13: R14: R15: [ 54.804652] FS: () GS:9905dfb8() knlGS:
Re: [PATCH] MAINTAINERS: update entries for ARM/STM32
2018-02-26 14:03 GMT+01:00 Alexandre Torgue : > Changes old git repository to the maintained one and adds more patterns. > > Signed-off-by: Alexandre Torgue > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3bdc260..802b984 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1999,8 +1999,10 @@ M: Maxime Coquelin > M: Alexandre Torgue > L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) > S: Maintained > -T: git git://git.kernel.org/pub/scm/linux/kernel/git/mcoquelin/stm32.git > +T: git git://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git > stm32-next > N: stm32 > +F: arch/arm/boot/dts/stm32* > +F: arch/arm/mach-stm32/ > F: drivers/clocksource/armv7m_systick.c > > ARM/TANGO ARCHITECTURE > -- > 2.7.4 > Acked-by: Maxime Coquelin Thanks, Maxime
Re: [PATCH v2] irqchip: stm32: Fix copyright
2017-11-30 9:45 GMT+01:00 Benjamin Gaignard : > Uniformize STMicroelectronics copyrights header > Add SPDX identifier > > Signed-off-by: Benjamin Gaignard > Acked-by: Alexandre TORGUE > CC: Maxime Coquelin > --- > drivers/irqchip/irq-stm32-exti.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/irqchip/irq-stm32-exti.c > b/drivers/irqchip/irq-stm32-exti.c > index 31ab0dee2ce7..36f0fbe36c35 100644 > --- a/drivers/irqchip/irq-stm32-exti.c > +++ b/drivers/irqchip/irq-stm32-exti.c > @@ -1,7 +1,8 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > * Copyright (C) Maxime Coquelin 2015 > + * Copyright (C) STMicroelectronics 2017 > * Author: Maxime Coquelin > - * License terms: GNU General Public License (GPL), version 2 > */ > > #include > -- > 2.15.0 > Acked-by: Maxime Coquelin Maxime
Re: [PATCH v2] pinctrl: stm32: Fix copyright
2017-11-30 9:46 GMT+01:00 Benjamin Gaignard : > Uniformize STMicroelectronics copyrights header > Add SPDX identifier > > Signed-off-by: Benjamin Gaignard > Acked-by: Alexandre TORGUE > CC: Maxime Coquelin > --- > drivers/pinctrl/stm32/pinctrl-stm32.c | 3 ++- > drivers/pinctrl/stm32/pinctrl-stm32.h | 3 ++- > drivers/pinctrl/stm32/pinctrl-stm32f429.c | 3 ++- > drivers/pinctrl/stm32/pinctrl-stm32f469.c | 6 +++--- > drivers/pinctrl/stm32/pinctrl-stm32f746.c | 3 ++- > drivers/pinctrl/stm32/pinctrl-stm32h743.c | 6 +++--- > 6 files changed, 14 insertions(+), 10 deletions(-) Acked-by: Maxime Coquelin Maxime
Re: [PATCH v4] arch: arm: mach-stm32: Fix copyright
2017-11-30 9:49 GMT+01:00 Benjamin Gaignard : > Uniformize STMicroelectronics copyrights header > Add SPDX identifier > > Signed-off-by: Benjamin Gaignard > CC: Maxime Coquelin > --- > arch/arm/mach-stm32/board-dt.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-stm32/board-dt.c b/arch/arm/mach-stm32/board-dt.c > index e918686e4191..548a19f97129 100644 > --- a/arch/arm/mach-stm32/board-dt.c > +++ b/arch/arm/mach-stm32/board-dt.c > @@ -1,7 +1,8 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > * Copyright (C) Maxime Coquelin 2015 > + * Copyright (C) STMicroelectronics 2017 > * Author: Maxime Coquelin > - * License terms: GNU General Public License (GPL), version 2 > */ > > #include > -- > 2.15.0 > Acked-by: Maxime Coquelin Maxime
Re: [PATCH v6 2/4] drivers: irqchip: Add STM32 external interrupts support
2016-09-21 9:50 GMT+02:00 Thomas Gleixner : > On Wed, 21 Sep 2016, Alexandre Torgue wrote: > >> Hi Thomas, >> >> On 09/20/2016 10:16 PM, Thomas Gleixner wrote: >> > Alexandre, >> > >> > On Tue, 20 Sep 2016, Alexandre TORGUE wrote: >> > >> > > The STM32 external interrupt controller consists of edge detectors that >> > > generate interrupts requests or wake-up events. >> > > >> > > Each line can be independently configured as interrupt or wake-up source, >> > > and triggers either on rising, falling or both edges. Each line can also >> > > be masked independently. >> > > >> > > Signed-off-by: Maxime Coquelin >> > > Signed-off-by: Alexandre TORGUE >> > >> > That all looks very reasonable now. The only remaining question is your SOB >> > chain. Who is the author of these patches? You or Maxime? If it's Maxime, >> > then the changelog misses a From: tag. If it's you then Maximes SOB is >> > bogus. >> >> Actually Maxime wrote the main part of this driver and sent version 1 and 2 >> of >> the series. After Linus W. reviews, rework was required to use hierarchical >> domain. According to Maxime, I coded the rework (adaptation to hierarchical >> domain) and sent other version of the series. > > So I replace Maximes SOB with Originally-by: Does that apply to all four > patches? Yes, that's fine. Alex did a lot of rework on this series, he deserves the SoB. Thanks, Maxime
Re: [PATCH v2 07/10] reset: stm32: add driver Kconfig option
2016-08-30 10:24 GMT+02:00 Philipp Zabel : > Visible only if COMPILE_TEST is enabled, this allows to include the > driver in build tests. > > Cc: Maxime Coquelin > Cc: Gabriel Fernandez > Reviewed-by: Masahiro Yamada > Signed-off-by: Philipp Zabel > --- > drivers/reset/Kconfig | 6 ++ > drivers/reset/Makefile | 2 +- > 2 files changed, 7 insertions(+), 1 deletion(-) In line with what we did on other stm32 drivers: Reviewed-by: Maxime Coquelin Thanks! Maxime
[PATCH] MAINTAINERS: update STM32 maintainers list
I will have less time to work on STM32 platform, so I propose Alexandre as co-maintainer. Alex is working in the STMicroelectronics division in charge of STM32 family, so he will have access to all technical information and hardware. Signed-off-by: Maxime Coquelin Cc: Alexandre Torgue --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 7304d2e37a98..4504331809ac 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1723,6 +1723,7 @@ F:drivers/ata/ahci_st.c ARM/STM32 ARCHITECTURE M: Maxime Coquelin +M: Alexandre Torgue L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/mcoquelin/stm32.git -- 1.9.1
[PATCH] MAINTAINERS: update STi maintainer list
Remove myself as STi maintainer as I will no longer have access to STi platforms, and remove Srini too, who now works on other platforms. Patrice will manage the pull requests. Signed-off-by: Maxime Coquelin Cc: Patrice Chotard Cc: Srinivas Kandagatla Cc: Arnd Bergmann --- MAINTAINERS | 2 -- 1 file changed, 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 7304d2e37a98..1ea14565d9d8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1689,8 +1689,6 @@ S:Maintained F: drivers/edac/altera_edac. ARM/STI ARCHITECTURE -M: Srinivas Kandagatla -M: Maxime Coquelin M: Patrice Chotard L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) L: ker...@stlinux.com -- 1.9.1
Re: [PATCH V2 12/63] clocksource/drivers/armv7m_systick: Convert init function to return error
2016-06-16 23:26 GMT+02:00 Daniel Lezcano : > The init functions do not return any error. They behave as the following: > > - panic, thus leading to a kernel crash while another timer may work and >make the system boot up correctly > > or > > - print an error and let the caller unaware if the state of the system > > Change that by converting the init functions to return an error conforming > to the CLOCKSOURCE_OF_RET prototype. > > Proper error handling (rollback, errno value) will be changed later case > by case, thus this change just return back an error or success in the init > function. > > Signed-off-by: Daniel Lezcano > --- > drivers/clocksource/armv7m_systick.c | 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) Acked-by: Maxime Coquelin Thanks! Maxime
Re: [PATCH V2 08/63] clocksource/drivers/st_lpc: Convert init function to return error
On 06/16/2016 11:26 PM, Daniel Lezcano wrote: The init functions do not return any error. They behave as the following: - panic, thus leading to a kernel crash while another timer may work and make the system boot up correctly or - print an error and let the caller unaware if the state of the system Change that by converting the init functions to return an error conforming to the CLOCKSOURCE_OF_RET prototype. Proper error handling (rollback, errno value) will be changed later case by case, thus this change just return back an error or success in the init function. Signed-off-by: Daniel Lezcano --- drivers/clocksource/clksrc_st_lpc.c | 22 +- 1 file changed, 13 insertions(+), 9 deletions(-) Acked-by: Maxime Coquelin Thanks! Maxime
Re: [PATCH V2 18/63] clocksource/drivers/arm_global_timer: Convert init function to return error
On 06/16/2016 11:26 PM, Daniel Lezcano wrote: The init functions do not return any error. They behave as the following: - panic, thus leading to a kernel crash while another timer may work and make the system boot up correctly or - print an error and let the caller unaware if the state of the system Change that by converting the init functions to return an error conforming to the CLOCKSOURCE_OF_RET prototype. Proper error handling (rollback, errno value) will be changed later case by case, thus this change just return back an error or success in the init function. Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_global_timer.c | 28 ++-- 1 file changed, 18 insertions(+), 10 deletions(-) Acked-by: Maxime Coquelin Thanks! Maxime
Re: [PATCH V2 47/63] clocksource/drivers/timer-stm32: Convert init function to return error
2016-06-16 23:27 GMT+02:00 Daniel Lezcano : > The init functions do not return any error. They behave as the following: > > - panic, thus leading to a kernel crash while another timer may work and >make the system boot up correctly > > or > > - print an error and let the caller unaware if the state of the system > > Change that by converting the init functions to return an error conforming > to the CLOCKSOURCE_OF_RET prototype. > > Proper error handling (rollback, errno value) will be changed later case > by case, thus this change just return back an error or success in the init > function. > > Signed-off-by: Daniel Lezcano > --- > drivers/clocksource/timer-stm32.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) Acked-by: Maxime Coquelin Thanks! Maxime
[PATCH v4.7-rc] ARM: dts: STi: stih407-family: Disable reserved-memory co-processor nodes
From: Lee Jones
This patch fixes a non-booting issue in Mainline.
When booting with a compressed kernel, we need to be careful how we
populate memory close to DDR start. AUTO_ZRELADDR is enabled by default
in multi-arch enabled configurations, which place some restrictions on
where the kernel is placed and where it will be uncompressed to on boot.
AUTO_ZRELADDR takes the decompressor code's start address and masks out
the bottom 28 bits to obtain an address to uncompress the kernel to
(thus a load address of 0x4200 means that the kernel will be
uncompressed to 0x4000 i.e. DDR START on this platform).
Even changing the load address to after the co-processor's shared memory
won't render a booting platform, since the AUTO_ZRELADDR algorithm still
ensures the kernel is uncompressed into memory shared with the first
co-processor (0x4000).
Another option would be to move loading to 0x4A00, since this will
mean the decompressor will decompress the kernel to 0x4800. However,
this would mean a large chunk (0x4400 => 0x4800 (64MB)) of
memory would essentially be wasted for no good reason.
Until we can work with ST to find a suitable memory location to
relocate co-processor shared memory, let's disable the shared memory
nodes. This will ensure a working platform in the mean time.
NB: The more observant of you will notice that we're leaving the DMU
shared memory node enabled; this is because a) it is the only one in
active use at the time of this writing and b) it is not affected by
the current default behaviour which is causing issues.
Fixes: fe135c6 (ARM: dts: STiH407: Move over to using the 'reserved-memory' API
for obtaining DMA memory)
Signed-off-by: Lee Jones
Reviewed-by Peter Griffin
Signed-off-by: Maxime Coquelin
---
arch/arm/boot/dts/stih407-family.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/boot/dts/stih407-family.dtsi
b/arch/arm/boot/dts/stih407-family.dtsi
index ad8ba10764a3..d294e82447a2 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -24,18 +24,21 @@
compatible = "shared-dma-pool";
reg = <0x4000 0x0100>;
no-map;
+ status = "disabled";
};
gp1_reserved: rproc@4100 {
compatible = "shared-dma-pool";
reg = <0x4100 0x0100>;
no-map;
+ status = "disabled";
};
audio_reserved: rproc@4200 {
compatible = "shared-dma-pool";
reg = <0x4200 0x0100>;
no-map;
+ status = "disabled";
};
dmu_reserved: rproc@4300 {
--
1.9.1
Re: [PATCH] ARM: dts: STi: stih407-family: Disable reserved-memory co-processor nodes
On 06/07/2016 10:37 AM, Lee Jones wrote: This patch fixes a non-booting issue in Mainline. When booting with a compressed kernel, we need to be careful how we populate memory close to DDR start. AUTO_ZRELADDR is enabled by default in multi-arch enabled configurations, which place some restrictions on where the kernel is placed and where it will be uncompressed to on boot. AUTO_ZRELADDR takes the decompressor code's start address and masks out the bottom 28 bits to obtain an address to uncompress the kernel to (thus a load address of 0x4200 means that the kernel will be uncompressed to 0x4000 i.e. DDR START on this platform). Even changing the load address to after the co-processor's shared memory won't render a booting platform, since the AUTO_ZRELADDR algorithm still ensures the kernel is uncompressed into memory shared with the first co-processor (0x4000). Another option would be to move loading to 0x4A00, since this will mean the decompressor will decompress the kernel to 0x4800. However, this would mean a large chunk (0x4400 => 0x4800 (64MB)) of memory would essentially be wasted for no good reason. Until we can work with ST to find a suitable memory location to relocate co-processor shared memory, let's disable the shared memory nodes. This will ensure a working platform in the mean time. NB: The more observant of you will notice that we're leaving the DMU shared memory node enabled; this is because a) it is the only one in active use at the time of this writing and b) it is not affected by the current default behaviour which is causing issues. Fixes: fe135c6 (ARM: dts: STiH407: Move over to using the 'reserved-memory' API for obtaining DMA memory) Signed-off-by: Lee Jones --- arch/arm/boot/dts/stih407-family.dtsi | 3 +++ 1 file changed, 3 insertions(+) That sounds reasonable: Acked-by: Maxime Coquelin Arnd, Olof, can you pick this patch directly in -rc fixes, as it prevents STi boards to boot correctly? Thanks in advance, Maxime
Re: [PATCH v2 3/5] ARM: dts: Add I2C1 support for STM32F429 SoC
2016-06-02 16:26 GMT+02:00 M'boumba Cedric Madianga :
> Signed-off-by: Patrice Chotard
> Signed-off-by: M'boumba Cedric Madianga
> ---
> arch/arm/boot/dts/stm32f429.dtsi | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
> b/arch/arm/boot/dts/stm32f429.dtsi
> index 434d4b9..d5857eb 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -323,6 +323,18 @@
> slew-rate = <2>;
> };
> };
> +
> + i2c1_sda_pin: i2c1_sda@0 {
> + pins {
> + pinmux =
> ;
> + drive-open-drain;
> + };
> + };
> + i2c1_scl_pin: i2c1_scl@0 {
> + pins {
> + pinmux =
> ;
> + };
> + };
> };
Shouldn't be preferrable to group the two functions in a single one,
as done for usart config in this file?:
usart1_pins_a: usart1@0 {
pins1 {
pinmux = ;
bias-disable;
drive-push-pull;
slew-rate = <0>;
};
pins2 {
pinmux = ;
bias-disable;
};
};
Also, I would prefer the phandle to be suffixed with the port number,
as multiple muxing options are available, for example "i2c1_pins_b".
Thanks,
Maxime
Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver
Hi Cedric,
2016-06-02 17:35 GMT+02:00 M'boumba Cedric Madianga :
> Hi,
>
>>> +
>>> +/**
>>> + * stm32f4_i2c_xfer() - Transfer combined I2C message
>>> + * @i2c_adap: Adapter pointer to the controller
>>> + * @msgs: Pointer to data to be written.
>>> + * @num: Number of messages to be executed
>>> + */
>>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg
>>> msgs[],
>>> + int num)
>>> +{
>>> + struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>>> + int ret, i;
>>> +
>>> + i2c_dev->busy = true;
>>> +
>>> + ret = clk_prepare_enable(i2c_dev->clk);
>>> + if (ret) {
>>> + dev_err(i2c_dev->dev, "Failed to prepare_enable clock\n");
>>> + return ret;
>>> + }
>>> +
>>> + stm32f4_i2c_hw_config(i2c_dev);
>> Maybe you could call this only at probe and resume time?
>> You would save some register accesses.
> Some clarification about this point.
> We need to call stm32f4_i2c_hw_config before each I2C transfer as at
> the end of I2C communication the peripheral is automatically disabled
> and configuration registers are reset.
Ok, but I wonder how the IP knows this is the last i2c message to be sent?
Or maybe it gets re-initialized as soon as the clk is disabled?
Thanks for the inputs,
Maxime
Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver
2016-06-01 16:01 GMT+02:00 M'boumba Cedric Madianga :
> Hi Maxime,
>
+static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
+{
+ struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
+ u32 ccr, val, clk_rate;
+
+ ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
+ ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
+STM32F4_I2C_CCR_CCR_MASK);
+
+ clk_rate = clk_get_rate(i2c_dev->clk);
+
+ switch (i2c_dev->speed) {
+ case STM32F4_I2C_SPEED_STANDARD:
+ val = clk_rate / t->rate * 2;
+ if (val < STM32F4_I2C_MIN_CCR)
+ ccr |= STM32F4_I2C_CCR_CCR(STM32F4_I2C_MIN_CCR);
+ else
+ ccr |= STM32F4_I2C_CCR_CCR(val);
+ break;
+ case STM32F4_I2C_SPEED_FAST:
+ ccr |= STM32F4_I2C_CCR_FS;
+ if (t->duty) {
+ ccr |= STM32F4_I2C_CCR_DUTY;
+ ccr |= STM32F4_I2C_CCR_CCR(clk_rate / t->rate *
25);
+ } else {
+ ccr |= STM32F4_I2C_CCR_CCR(clk_rate / t->rate * 3);
+ }
>>> Is it really useful since duty seems to always be 0?
>> Agree, I will rework it by directly set duty at 0 in the register.
>
> Contrary to what I wrote previously, the duty has to be set for FAST
> Mode to reach 400khz.
> So, I am going to keep the timing struct and set duty to 1 for FAST mode
Ok, That's fine by me.
[PATCH] hwrng: stm32: fix maybe uninitialized variable warning
This patch fixes the following warning:
drivers/char/hw_random/stm32-rng.c: In function 'stm32_rng_read':
drivers/char/hw_random/stm32-rng.c:82:19: warning: 'sr' may be used
uninitialized in this function
Reported-by: Sudip Mukherjee
Suggested-by: Arnd Bergmann
Cc: Daniel Thompson
Signed-off-by: Maxime Coquelin
---
drivers/char/hw_random/stm32-rng.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/char/hw_random/stm32-rng.c
b/drivers/char/hw_random/stm32-rng.c
index 92a810648bd0..63d84e6f1891 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -69,8 +69,12 @@ static int stm32_rng_read(struct hwrng *rng, void *data,
size_t max, bool wait)
}
/* If error detected or data not ready... */
- if (sr != RNG_SR_DRDY)
+ if (sr != RNG_SR_DRDY) {
+ if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
+ "bad RNG status - %x\n", sr))
+ writel_relaxed(0, priv->base + RNG_SR);
break;
+ }
*(u32 *)data = readl_relaxed(priv->base + RNG_DR);
@@ -79,10 +83,6 @@ static int stm32_rng_read(struct hwrng *rng, void *data,
size_t max, bool wait)
max -= sizeof(u32);
}
- if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
- "bad RNG status - %x\n", sr))
- writel_relaxed(0, priv->base + RNG_SR);
-
pm_runtime_mark_last_busy((struct device *) priv->rng.priv);
pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv);
--
1.9.1
Re: [PATCH v2] pinctrl: stm32: factorize stm32_pconf_input/output_get()
On 05/24/2016 01:57 PM, patrice.chot...@st.com wrote: From: Patrice Chotard As these 2 functions code are 95% similar, factorize them. Signed-off-by: Patrice Chotard --- drivers/pinctrl/stm32/pinctrl-stm32.c | 31 ++- 1 file changed, 10 insertions(+), 21 deletions(-) Acked-by: Maxime Coquelin Thanks! Maxime
Re: [PATCH] hwrng: stm32 - fix build warning
2016-05-24 12:09 GMT+02:00 Daniel Thompson : > On 24/05/16 09:50, Maxime Coquelin wrote: >> >> diff --git a/drivers/char/hw_random/stm32-rng.c >> b/drivers/char/hw_random/stm32-rng.c >> index 92a810648bd0..2a0fc90e4dc3 100644 >> --- a/drivers/char/hw_random/stm32-rng.c >> +++ b/drivers/char/hw_random/stm32-rng.c >> @@ -68,6 +68,10 @@ static int stm32_rng_read(struct hwrng *rng, void >> *data, size_t max, bool wait) >> } while (!sr && --timeout); >> } >> >> + if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS), >> + "bad RNG status - %x\n", sr)) >> + writel_relaxed(0, priv->base + RNG_SR); >> + >> /* If error detected or data not ready... */ >> if (sr != RNG_SR_DRDY) >> break; > > > Minor quibble but I might prefer that the error handling/recovery actually > be put on the error path itself (included in the if (sr != RNG_SR_DRDY) ). Yes, it would be better. Regards, Maxime
Re: [PATCH] hwrng: stm32 - fix build warning
2016-05-24 10:58 GMT+02:00 Arnd Bergmann : > On Tuesday, May 24, 2016 10:50:17 AM CEST Maxime Coquelin wrote: >> diff --git a/drivers/char/hw_random/stm32-rng.c >> b/drivers/char/hw_random/stm32-rng.c >> index 92a810648bd0..2a0fc90e4dc3 100644 >> --- a/drivers/char/hw_random/stm32-rng.c >> +++ b/drivers/char/hw_random/stm32-rng.c >> @@ -68,6 +68,10 @@ static int stm32_rng_read(struct hwrng *rng, void >> *data, size_t max, bool wait) >> } while (!sr && --timeout); >> } >> >> + if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS), >> + "bad RNG status - %x\n", sr)) >> + writel_relaxed(0, priv->base + RNG_SR); >> + >> /* If error detected or data not ready... */ >> if (sr != RNG_SR_DRDY) >> break; >> @@ -79,10 +83,6 @@ static int stm32_rng_read(struct hwrng *rng, void >> *data, size_t max, bool wait) >> max -= sizeof(u32); >> } >> >> - if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS), >> - "bad RNG status - %x\n", sr)) >> - writel_relaxed(0, priv->base + RNG_SR); >> - >> pm_runtime_mark_last_busy((struct device *) priv->rng.priv); >> pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv); >> >> Thanks, >> > > Yes, that looks good to me. Thanks! Sudip, do you want to send the patch, or I manage to do it? Maxime
Re: [PATCH] hwrng: stm32 - fix build warning
2016-05-24 10:32 GMT+02:00 Arnd Bergmann :
> On Tuesday, May 24, 2016 9:59:41 AM CEST Maxime Coquelin wrote:
>> 2016-05-23 22:35 GMT+02:00 Arnd Bergmann :
>> > On Monday, May 23, 2016 6:14:08 PM CEST Sudip Mukherjee wrote:
>> >> diff --git a/drivers/char/hw_random/stm32-rng.c
>> >> b/drivers/char/hw_random/stm32-rng.c
>> >> index 92a8106..0533370 100644
>> >> --- a/drivers/char/hw_random/stm32-rng.c
>> >> +++ b/drivers/char/hw_random/stm32-rng.c
>> >> @@ -52,7 +52,7 @@ static int stm32_rng_read(struct hwrng *rng, void
>> >> *data, size_t max, bool wait)
>> >> {
>> >> struct stm32_rng_private *priv =
>> >> container_of(rng, struct stm32_rng_private, rng);
>> >> - u32 sr;
>> >> + u32 sr = 0;
>> >> int retval = 0;
>> >>
>> >> pm_runtime_get_sync((struct device *) priv->rng.priv);
>> >
>> > Does this work as well?
>> >
>> > diff --git a/drivers/char/hw_random/stm32-rng.c
>> > b/drivers/char/hw_random/stm32-rng.c
>> > index 92a810648bd0..5c836b0afa40 100644
>> > --- a/drivers/char/hw_random/stm32-rng.c
>> > +++ b/drivers/char/hw_random/stm32-rng.c
>> > @@ -79,7 +79,7 @@ static int stm32_rng_read(struct hwrng *rng, void *data,
>> > size_t max, bool wait)
>> > max -= sizeof(u32);
>> > }
>> >
>> > - if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
>> > + if (WARN_ONCE(retval > 0 && (sr & (RNG_SR_SEIS | RNG_SR_CEIS)),
>> > "bad RNG status - %x\n", sr))
>> > writel_relaxed(0, priv->base + RNG_SR);
>> >
>> > I think it would be nicer to not add a bogus initialization.
>> Hmm, no sure this nicer.
>> The while loop can break before retval is incremented when sr value is
>> not expected (sr != RNG_SR_DRDY).
>> In that case, we certainly want to print sr value.
>
> Ah, you are right.
>
>> Maybe the better way is just to initialize sr with status register content?
>
>>pm_runtime_get_sync((struct device *) priv->rng.priv);
>>
>>+ sr = readl_relaxed(priv->base + RNG_SR);
>>while (max > sizeof(u32)) {
>>- sr = readl_relaxed(priv->base + RNG_SR);
>>if (!sr && wait) {
>>unsigned int timeout = RNG_TIMEOUT;
>
>
> I think that introduces a bug: you really want to read the status
> register on each loop iteration.
Actually, I read the status again at the end of the loop.
But my implementation isn't good anyway, because I read the status
register one time more every time.
>
> How about moving the error handling into the loop itself?
That would be better, indeed, but there is one problem with your below proposal:
>
> Arnd
>
>
> diff --git a/drivers/char/hw_random/stm32-rng.c
> b/drivers/char/hw_random/stm32-rng.c
> index 92a810648bd0..fceacd809462 100644
> --- a/drivers/char/hw_random/stm32-rng.c
> +++ b/drivers/char/hw_random/stm32-rng.c
> @@ -59,6 +59,10 @@ static int stm32_rng_read(struct hwrng *rng, void *data,
> size_t max, bool wait)
>
> while (max > sizeof(u32)) {
> sr = readl_relaxed(priv->base + RNG_SR);
> + if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
> + "bad RNG status - %x\n", sr))
> + writel_relaxed(0, priv->base + RNG_SR);
> +
The error handling should be moved after the last status register read.
> if (!sr && wait) {
> unsigned int timeout = RNG_TIMEOUT;
>
> @@ -79,10 +83,6 @@ static int stm32_rng_read(struct hwrng *rng, void *data,
> size_t max, bool wait)
> max -= sizeof(u32);
> }
>
> - if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
> - "bad RNG status - %x\n", sr))
> - writel_relaxed(0, priv->base + RNG_SR);
> -
> pm_runtime_mark_last_busy((struct device *) priv->rng.priv);
> pm_runtime_put_sync_autosuspend((struct device *) priv->rng.priv);
diff --git a/drivers/char/hw_random/stm32-rng.c
b/drivers/char/hw_random/stm32-rng.c
index 92a810648bd0..2a0fc90e4dc3 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -68,6 +68,10 @@ static int stm32_rng_read(struct hwrng *rng, void
*data, size_t max, bool wait)
} while (!sr && --timeout);
}
+
Re: [PATCH] hwrng: stm32 - fix build warning
2016-05-23 22:35 GMT+02:00 Arnd Bergmann :
> On Monday, May 23, 2016 6:14:08 PM CEST Sudip Mukherjee wrote:
>> We have been getting build warning about:
>> drivers/char/hw_random/stm32-rng.c: In function 'stm32_rng_read':
>> drivers/char/hw_random/stm32-rng.c:82:19: warning: 'sr' may be used
>> uninitialized in this function
>>
>> On checking the code it turns out that sr can never be used
>> uninitialized as sr is getting initialized in the while loop and while
>> loop will always execute as the minimum value of max can be 32.
>> So just initialize sr to 0 while declaring it to silence the compiler.
>>
>> Signed-off-by: Sudip Mukherjee
>> ---
>
> I notice that you are using a really old compiler. While this warning
> seems to be valid in the sense that the compiler should figure out that
> the variable might be used uninitialized, please update your toolchain
> before reporting other such problems, as gcc-4.6 had a lot more false
> positives that newer ones (5.x or 6.x) have.
>
>>
>> build log at:
>> https://travis-ci.org/sudipm-mukherjee/parport/jobs/132180906
>>
>> drivers/char/hw_random/stm32-rng.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/hw_random/stm32-rng.c
>> b/drivers/char/hw_random/stm32-rng.c
>> index 92a8106..0533370 100644
>> --- a/drivers/char/hw_random/stm32-rng.c
>> +++ b/drivers/char/hw_random/stm32-rng.c
>> @@ -52,7 +52,7 @@ static int stm32_rng_read(struct hwrng *rng, void *data,
>> size_t max, bool wait)
>> {
>> struct stm32_rng_private *priv =
>> container_of(rng, struct stm32_rng_private, rng);
>> - u32 sr;
>> + u32 sr = 0;
>> int retval = 0;
>>
>> pm_runtime_get_sync((struct device *) priv->rng.priv);
>
> Does this work as well?
>
> diff --git a/drivers/char/hw_random/stm32-rng.c
> b/drivers/char/hw_random/stm32-rng.c
> index 92a810648bd0..5c836b0afa40 100644
> --- a/drivers/char/hw_random/stm32-rng.c
> +++ b/drivers/char/hw_random/stm32-rng.c
> @@ -79,7 +79,7 @@ static int stm32_rng_read(struct hwrng *rng, void *data,
> size_t max, bool wait)
> max -= sizeof(u32);
> }
>
> - if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
> + if (WARN_ONCE(retval > 0 && (sr & (RNG_SR_SEIS | RNG_SR_CEIS)),
> "bad RNG status - %x\n", sr))
> writel_relaxed(0, priv->base + RNG_SR);
>
> I think it would be nicer to not add a bogus initialization.
Hmm, no sure this nicer.
The while loop can break before retval is incremented when sr value is
not expected (sr != RNG_SR_DRDY).
In that case, we certainly want to print sr value.
Maybe the better way is just to initialize sr with status register content?
diff --git a/drivers/char/hw_random/stm32-rng.c
b/drivers/char/hw_random/stm32-rng.c
index 92a810648bd0..07a6659d0fe6 100644
--- a/drivers/char/hw_random/stm32-rng.c
+++ b/drivers/char/hw_random/stm32-rng.c
@@ -57,8 +57,8 @@ static int stm32_rng_read(struct hwrng *rng, void
*data, size_t max, bool wait)
pm_runtime_get_sync((struct device *) priv->rng.priv);
+ sr = readl_relaxed(priv->base + RNG_SR);
while (max > sizeof(u32)) {
- sr = readl_relaxed(priv->base + RNG_SR);
if (!sr && wait) {
unsigned int timeout = RNG_TIMEOUT;
@@ -77,6 +77,8 @@ static int stm32_rng_read(struct hwrng *rng, void
*data, size_t max, bool wait)
retval += sizeof(u32);
data += sizeof(u32);
max -= sizeof(u32);
+
+ sr = readl_relaxed(priv->base + RNG_SR);
}
if (WARN_ONCE(sr & (RNG_SR_SEIS | RNG_SR_CEIS),
Regards,
Maxime
Re: [RESEND PATCH v6 0/6] Add Ethernet support on STM32F429
2016-05-18 11:31 GMT+02:00 Arnd Bergmann : > On Wednesday 18 May 2016 09:48:53 Maxime Coquelin wrote: >> 2016-05-17 18:25 GMT+02:00 David Miller : >> > From: Maxime Coquelin >> > Date: Tue, 17 May 2016 11:20:16 +0200 >> > >> >> Hi David, >> >> >> >> 2016-05-09 21:06 GMT+02:00 David Miller : >> >>> From: Alexandre TORGUE >> >>> Date: Mon, 9 May 2016 12:31:33 +0200 >> >>> >> >>>> STM32F429 Chip embeds a Synopsys 3.50a MAC IP. >> >>>> This series: >> >>>> -enhance current stmmac driver to control it (code already >> >>>> available) and adds basic glue for STM32F429 chip. >> >>>> -Enable basic Net config in kernel. >> >>> >> >>> I assume this will go via the ARM tree, for the networking bits: >> >> I would expect patches 1, 2 & 3 to got via your tree, to avoid conflicts. >> > >> > I don't think putting them all via the ARM tree is going to create much >> > in the way of conflicts, and right now during the merge window offloading >> > that work from me would really help my backlog a lot. >> >> Ok, I understand this is not the best time to pick it. >> >> Arnd, Olof & Kevin, would you accept to pick the series in your tree? >> > > It's too late for v4.7 for us too, please pick up the arch/arm patches > in your normal stm32 tree and send them to us along with any other changes > you may have, and resend the driver by itself to netdev time after the > merge window. > > The binding document can go either way, with the dts changes or with > the driver. I see no dependencies between the patches, so we just need > to land them all in v4.8. Ok, this is good for me. No problem to wait for v4.8. I will take the arch/arm patches, and David the drivers/net/ ones once v4.7-rc1 is out. Thanks for your time, Maxime
Re: [RESEND PATCH v6 0/6] Add Ethernet support on STM32F429
2016-05-17 18:25 GMT+02:00 David Miller : > From: Maxime Coquelin > Date: Tue, 17 May 2016 11:20:16 +0200 > >> Hi David, >> >> 2016-05-09 21:06 GMT+02:00 David Miller : >>> From: Alexandre TORGUE >>> Date: Mon, 9 May 2016 12:31:33 +0200 >>> >>>> STM32F429 Chip embeds a Synopsys 3.50a MAC IP. >>>> This series: >>>> -enhance current stmmac driver to control it (code already >>>> available) and adds basic glue for STM32F429 chip. >>>> -Enable basic Net config in kernel. >>> >>> I assume this will go via the ARM tree, for the networking bits: >> I would expect patches 1, 2 & 3 to got via your tree, to avoid conflicts. > > I don't think putting them all via the ARM tree is going to create much > in the way of conflicts, and right now during the merge window offloading > that work from me would really help my backlog a lot. Ok, I understand this is not the best time to pick it. Arnd, Olof & Kevin, would you accept to pick the series in your tree? Regards, Maxime
Re: [PATCH 2/5] i2c: Add STM32F4 I2C driver
Hi Cedric,
2016-05-11 17:36 GMT+02:00 M'boumba Cedric Madianga :
> This patch adds support for the STM32F4 I2C controller.
>
> Signed-off-by: M'boumba Cedric Madianga
> ---
> drivers/i2c/busses/Kconfig | 10 +
> drivers/i2c/busses/Makefile | 1 +
> drivers/i2c/busses/i2c-stm32f4.c | 872
> +++
> 3 files changed, 883 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
>
...
> diff --git a/drivers/i2c/busses/i2c-stm32f4.c
> b/drivers/i2c/busses/i2c-stm32f4.c
> new file mode 100644
> index 000..4692213
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-stm32f4.c
...
> +enum stm32f4_i2c_speed {
> + STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
> + STM32F4_I2C_SPEED_FAST, /* 400 kHz */
> + STM32F4_I2C_SPEED_END,
> +};
> +
> +/**
> + * struct stm32f4_i2c_timings - per-Mode tuning parameters
> + * @rate: I2C bus rate
> + * @duty: Fast mode duty cycle
> + */
> +struct stm32f4_i2c_timings {
> + u32 rate;
> + u32 duty;
> +};
...
> +/**
> + * struct stm32f4_i2c_dev - private data of the controller
> + * @adap: I2C adapter for this controller
> + * @dev: device for this controller
> + * @base: virtual memory area
> + * @complete: completion of I2C message
> + * @irq_event: interrupt event line for the controller
> + * @irq_error: interrupt error line for the controller
> + * @clk: hw i2c clock
> + * speed: I2C clock frequency of the controller. Standard or Fast only
> supported
> + * @client: I2C transfer information
> + * @busy: I2C transfer on-going
> + * @rst: I2C reset line
> + */
> +struct stm32f4_i2c_dev {
> + struct i2c_adapter adap;
> + struct device *dev;
> + void __iomem*base;
> + struct completion complete;
> + int irq_event;
> + int irq_error;
> + struct clk *clk;
> + int speed;
> + struct stm32f4_i2c_client client;
> + boolbusy;
> + struct reset_control*rst;
> +};
> +
> +static struct stm32f4_i2c_timings i2c_timings[] = {
> + [STM32F4_I2C_SPEED_STANDARD] = {
> + .rate = 10,
> + .duty = 0,
> + },
> + [STM32F4_I2C_SPEED_FAST] = {
> + .rate = 40,
> + .duty = 0,
> + },
> +};
Is this table really needed?
As duty value seems to always be 0, couldn't you just store the rate
in the speed field?
> +static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + u32 trise, freq, cr2;
> +
> + cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> + freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
> +
> + trise = readl_relaxed(i2c_dev->base + STM32F4_I2C_TRISE);
> + trise &= ~STM32F4_I2C_TRISE_TRISE_MASK;
> +
> + if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
> + trise |= STM32F4_I2C_TRISE_TRISE((freq + 1));
> + else
> + trise |= STM32F4_I2C_TRISE_TRISEfreq * 300) / 1000) + 1));
Or if you keep the timings struct, maybe the rise time should be part of it.
Doing, that, you will avoid the if/else
> +
> + writel_relaxed(trise, i2c_dev->base + STM32F4_I2C_TRISE);
> +}
> +
> +static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
> + u32 ccr, val, clk_rate;
> +
> + ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
> + ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
> +STM32F4_I2C_CCR_CCR_MASK);
> +
> + clk_rate = clk_get_rate(i2c_dev->clk);
> +
> + switch (i2c_dev->speed) {
> + case STM32F4_I2C_SPEED_STANDARD:
> + val = clk_rate / t->rate * 2;
> + if (val < STM32F4_I2C_MIN_CCR)
> + ccr |= STM32F4_I2C_CCR_CCR(STM32F4_I2C_MIN_CCR);
> + else
> + ccr |= STM32F4_I2C_CCR_CCR(val);
> + break;
> + case STM32F4_I2C_SPEED_FAST:
> + ccr |= STM32F4_I2C_CCR_FS;
> + if (t->duty) {
> + ccr |= STM32F4_I2C_CCR_DUTY;
> + ccr |= STM32F4_I2C_CCR_CCR(clk_rate / t->rate * 25);
> + } else {
> + ccr |= STM32F4_I2C_CCR_CCR(clk_rate / t->rate * 3);
> + }
Is it really useful since duty seems to always be 0?
> + break;
> + default:
> + dev_err(i2c_dev->dev, "I2C speed mode not supported\n");
> + }
> +
> + writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
> +}
> +
> +static void stm32f4_i2c_set_filter(struct stm32f4_i2c_dev *i2c_dev)
> +{
> + u32 filter;
> +
> + /* Enable analog noise filter
Re: [RESEND PATCH v6 0/6] Add Ethernet support on STM32F429
Hi David, 2016-05-09 21:06 GMT+02:00 David Miller : > From: Alexandre TORGUE > Date: Mon, 9 May 2016 12:31:33 +0200 > >> STM32F429 Chip embeds a Synopsys 3.50a MAC IP. >> This series: >> -enhance current stmmac driver to control it (code already >> available) and adds basic glue for STM32F429 chip. >> -Enable basic Net config in kernel. > > I assume this will go via the ARM tree, for the networking bits: I would expect patches 1, 2 & 3 to got via your tree, to avoid conflicts. > Acked-by: David S. Miller Thanks, Maxime
Re: [PATCH v2 6/9] pinctrl: Add IRQ support to STM32 gpios
2016-04-30 13:32 GMT+02:00 Linus Walleij : > On Fri, Apr 29, 2016 at 1:19 PM, Maxime Coquelin > wrote: >> It looks like this: http://pastebin.com/raw/cs2WiNKZ >> You can directly check section 12.2.5 of the stm32f429 reference manual: >> http://www2.st.com/resource/en/reference_manual/dm00031020.pdf > > Nice ASCII art, include that into the commit message :) I will! :) > >> For example, we can imagine a board where gpioa0 is used SD's card detect, >> and gpiob0 used to control a led. >> >> If we set the mux at request time, gpiob0 request may overwrite the mux >> configuration set by gpioa0, whereas it is not used as interrupt. >> >> That is the reason why I did it in .to_irq(). > > Well now I am even *more* convinced that this should not happen in > .to_irq(). .to_irq() should not do *anything*. > > This needs to happen as part of the irqchip setting up the actual > interrupt. > > And it seems the problem is that this driver does *not* define its > own irqchip, but it *should*. > > What you want to do is implement an hierarchical irqdomain in your > irqchip, which is what other drivers of this type are doing, see: > drivers/gpio/gpio-xgene-sb.c > irq_domain_create_hierarchy() is your friend. > >>> It should *also* be done in the set-up path for the irqchip >>> side, if that line is used without any interaction with the >>> gpio side of things. >> >> Actually, a line is either used by a GPIO, (exclusive) or another purpose. >> In the case of a GPIO line, I think we should always go through the gpio. > > This is a textbook example of a hierarchichal irq domain I think. > >>> The point is that each IRQ that ever get used >>> has a 1-to-1 relationship to a certain GPIO line, and if that >>> relationship cannot be resolved from the irqchip side, >>> something is wrong. The irqchip needs to enable the >>> GPIO line it is backing to recieve interrupts without any >>> requirements that .to_irq() have been called first. >> >> Actually, this is not a 1:1 relationship, as for example, exti0 can be >> connected >> to either gpioa0, or gpiob0,..., or gpiok0. > > That is what hierarchical irqdomain is for. > > You should expose an irqchip from the gpio driver, that give you > unique irq translations for *all* GPIO lines. > > Then, at runtime, if the hierarchical irqdomain cannot map > (i.e. mux) the interrupt onto one of the available lines, > you will get an error. Thanks a lot for this valuable feedback. I will have a look at hierachical irq domains, and hope to come back this week with an implementation. Regards, Maxime
Re: [PATCH] i2c: st: Implement i2c_bus_recovery_info callbacks
On 04/28/2016 04:57 PM, Wolfram Sang wrote: The trick is to switch to SPI mode, 9 bits words and write a 0, so that 9 clock pulses are generated. Heh. As long as it works :) But as you said, it really needs a comment. :) I didn't faced the problem myself, but it looks good with an oscilloscope, and a customer reported it to work. Peter, will you resend with adding the explanations I provided as comment? Thanks in advance, Maxime
Re: [PATCH v6 2/6] Documentation: Bindings: Add STM32 DWMAC glue
2016-04-28 22:59 GMT+02:00 Rob Herring : > On Mon, Apr 25, 2016 at 01:53:58PM +0200, Alexandre TORGUE wrote: >> Signed-off-by: Alexandre TORGUE > > Acked-by: Rob Herring Thanks Rob! Arnd, I only have patches 4, 5 and 6 of this series for stm32 (2 DT, one defconfig) for v4.7. Should I send a pull request, or I can send the patches directly to a...@kernel.org so that you apply them directly? Thanks in advance, Maxime
Re: [PATCH v2 6/9] pinctrl: Add IRQ support to STM32 gpios
2016-04-29 10:53 GMT+02:00 Linus Walleij :
> On Tue, Apr 19, 2016 at 11:04 AM, Maxime Coquelin
> wrote:
>> 2016-04-08 11:43 GMT+02:00 Linus Walleij :
>>> On Thu, Mar 31, 2016 at 5:09 PM, Maxime Coquelin
>>> wrote:
>>>
>>>> +static int stm32_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>>>> +{
>>>> + struct stm32_pinctrl *pctl = dev_get_drvdata(chip->parent);
>>>> + struct stm32_gpio_bank *bank = gpiochip_get_data(chip);
>>>> + unsigned int irq;
>>>> +
>>>> + regmap_field_write(pctl->irqmux[offset], bank->range.id);
>>>
>>> No. You must implement the irqchip and GPIO controllers to
>>> be orthogonal, doing things like this creates a semantic that
>>> assumes .to_irq() is always called before using the IRQ and
>>> that is not guaranteed at all. A consumer may very well
>>> use an interrupt right off the irqchip without this being called
>>> first. All this function should do is translate a number. No
>>> other semantics.
>>>
>>> This needs to be done from the irqchip (sorry).
>>
>> Actually, the register written here is not part of the irqchip.
>> It is a system config register that is only used when using a GPIO as
>> external interrupt.
>> Its aim is to mux the GPIO bank on a line.
>
> Then it should be done in .request() for the GPIO, not in
> .to_irq().
Problem is that at request time, we don't know whether the GPIO is to
be used as an interrupt or not.
I think I may have not been clear enough in the HW architecture description.
Indeed, I used the term "mux", but should instead use the term "selector".
The idea is that between the GPIO controllers and the EXTI controller,
there is one selector for each line number, to select the controller used as
interrupt source.
For example, there is a selector on line 0 to select between gpioa0, gpiob0,
gpioc0,.., gpiok0, which one is connected to exti0.
It means there is a HW restriction that makes impossible to use both GPIOA0
or GPIOB0 as interrupts at the same time.
It looks like this: http://pastebin.com/raw/cs2WiNKZ
You can directly check section 12.2.5 of the stm32f429 reference manual:
http://www2.st.com/resource/en/reference_manual/dm00031020.pdf
For example, we can imagine a board where gpioa0 is used SD's card detect,
and gpiob0 used to control a led.
If we set the mux at request time, gpiob0 request may overwrite the mux
configuration set by gpioa0, whereas it is not used as interrupt.
That is the reason why I did it in .to_irq().
>
> It should *also* be done in the set-up path for the irqchip
> side, if that line is used without any interaction with the
> gpio side of things.
Actually, a line is either used by a GPIO, (exclusive) or another purpose.
In the case of a GPIO line, I think we should always go through the gpio.
>
>> For example on line 1, it can be muxed to select either gpioa1,
>> gpiob1, gpioc1, ...
>> How could I do it in the irqchip that has no gpio knowledge?
>
> I don't understand this.
>
> We are discussing an irqchip that is tightly coupled with
> a gpiochip. Usually d->hwirq is the same as the GPIO offset
> but that varies.
>
> The point is that each IRQ that ever get used
> has a 1-to-1 relationship to a certain GPIO line, and if that
> relationship cannot be resolved from the irqchip side,
> something is wrong. The irqchip needs to enable the
> GPIO line it is backing to recieve interrupts without any
> requirements that .to_irq() have been called first.
Actually, this is not a 1:1 relationship, as for example, exti0 can be connected
to either gpioa0, or gpiob0,..., or gpiok0.
So for the exti entries that are connected to gpios, I don't see how
we can reference it
from the exti controller,
>
> If to_irq() does something else than translate to an IRQ
> something is wrong.
Ok, so maybe we need an intermediate layer between gpio and exti to manage the
selectors sysconf?
I wiil have a look in that direction, but I think it could be over-engineered.
Thanks!
Maxime
Re: [PATCH] i2c: st: Implement i2c_bus_recovery_info callbacks
Hi Wolfram,
On 04/24/2016 11:10 PM, Wolfram Sang wrote:
+/*
+ * i2c bus recovery routines
+ * get_scl and set_scl must be defined to avoid the recover_bus field of
+ * i2c_bus_recovery_info to be overriden with NULL during the
+ * i2c_add_adapter call
+ */
Oh, that shouldn't be like this. Can you try this patch and remove the
empty functions please?
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 4979728f7fb2de..604936955807e5 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1595,10 +1595,12 @@ static int i2c_register_adapter(struct i2c_adapter
*adap)
bri->get_scl = get_scl_gpio_value;
bri->set_scl = set_scl_gpio_value;
- } else if (!bri->set_scl || !bri->get_scl) {
+ } else if (bri->recover_bus == i2c_generic_scl_recovery) {
/* Generic SCL recovery */
- dev_err(&adap->dev, "No {get|set}_gpio() found, not using
recovery\n");
- adap->bus_recovery_info = NULL;
+ if (!bri->set_scl || !bri->get_scl) {
+ dev_err(&adap->dev, "No {get|set}_scl() found, not
using recovery\n");
+ adap->bus_recovery_info = NULL;
+ }
}
}
+static int st_i2c_recover_bus(struct i2c_adapter *i2c_adap)
+{
Can you describe what the function does? It is not clear to me that it
generates 9 scl pulses.
I agree, it would need some comments.
This IP is dual-role, it can do either SPI or I2C.
The trick is to switch to SPI mode, 9 bits words and write a 0,
so that 9 clock pulses are generated.
This is easier to manage than switching to GPIO mode,
as we don't need to provide the gpio handles in DT, and no need to
put/get the pinctrl handle.
Regards,
Maxime
Re: [PATCH v6 6/6] ARM: dts: stm32f429: Update Ethernet node on Eval board
2016-04-25 13:54 GMT+02:00 Alexandre TORGUE : > Update new pinctrl phandle name and use new node name. > > Signed-off-by: Alexandre TORGUE > > diff --git a/arch/arm/boot/dts/stm32429i-eval.dts > b/arch/arm/boot/dts/stm32429i-eval.dts Acked-by: Maxime Coquelin Thanks! Maxime
Re: [PATCH v6 5/6] ARM: dts: stm32f429: Align Ethernet node with new bindings properties
Hi Alex,
2016-04-25 13:54 GMT+02:00 Alexandre TORGUE :
> This patch aligns clocks names and node reference according to new
> stm32-dwmac glue binding. It also renames Ethernet pinctrl phandle
> (indeed there is no need to add 0 as Ethernet instance as there is only
> one IP in SOC).
>
> Signed-off-by: Alexandre TORGUE
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi
> b/arch/arm/boot/dts/stm32f429.dtsi
> index 35df462..5995998 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -304,7 +304,7 @@
> };
> };
>
> - ethernet0_mii: mii@0 {
> + ethernet_mii: mii@0 {
> pins {
> pinmux =
> ,
>
> ,
> @@ -363,13 +363,13 @@
> st,mem2mem;
> };
>
> - ethernet0: dwmac@40028000 {
> + mac: ethernet@40028000 {
> compatible = "st,stm32-dwmac", "snps,dwmac-3.50a";
> reg = <0x40028000 0x8000>;
> reg-names = "stmmaceth";
> interrupts = <61>, <62>;
> interrupt-names = "macirq", "eth_wake_irq";
> - clock-names = "stmmaceth", "tx-clk", "rx-clk";
> + clock-names = "stmmaceth", "mac-clk-tx", "mac-clk-rx";
It looks good to me, but I will wait for Rob's ack on the bindings
documentation patch before applying it.
Regards,
Maxime
Re: [PATCH v6 4/6] ARM: STM32: Enable Ethernet in stm32_defconfig
Hi Alex, 2016-04-25 13:54 GMT+02:00 Alexandre TORGUE : > Enable basic Ethernet support (IPV4) for stm32 defconfig. > > Signed-off-by: Alexandre TORGUE Acked-by: Maxime Coquelin Thanks! Maxime
Re: [[PATCH v2] 1/8] ARM: dts: STi: STiH407: Provide generic (safe) DVFS configuration
Hi Lee,
Series is applied, thanks for having done the changes.
Note that I had to rework all the commit titles, because of the
double brackets ("[[PATCH v2] 1/8]").
Something wrong with your script?
Thanks,
Maxime
On 04/21/2016 05:07 PM, Lee Jones wrote:
You'll notice that the voltage cell is populated with 0's. Voltage
information is very platform specific, even depends on 'cut' and
'substrate' versions. Thus it is left blank for a generic (safe)
implementation. If other nodes/properties are provided by the
bootloader, the ST CPUFreq driver will over-ride these generic
values.
Signed-off-by: Lee Jones
Signed-off-by: Maxime Coquelin
---
arch/arm/boot/dts/stih407-family.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/stih407-family.dtsi
b/arch/arm/boot/dts/stih407-family.dtsi
index 81f8121..9fa1e58 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -22,15 +22,29 @@
device_type = "cpu";
compatible = "arm,cortex-a9";
reg = <0>;
+
/* u-boot puts hpen in SBC dmem at 0xa4 offset */
cpu-release-addr = <0x94100A4>;
+
+/* kHz uV */
+ operating-points = <150 0
+ 120 0
+ 80 0
+ 50 0>;
};
cpu@1 {
device_type = "cpu";
compatible = "arm,cortex-a9";
reg = <1>;
+
/* u-boot puts hpen in SBC dmem at 0xa4 offset */
cpu-release-addr = <0x94100A4>;
+
+/* kHz uV */
+ operating-points = <150 0
+ 120 0
+ 80 0
+ 50 0>;
};
};
Re: [PATCH v2 6/9] pinctrl: Add IRQ support to STM32 gpios
Hi Linus
2016-04-08 11:43 GMT+02:00 Linus Walleij :
> On Thu, Mar 31, 2016 at 5:09 PM, Maxime Coquelin
> wrote:
>
>> +static int stm32_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>> +{
>> + struct stm32_pinctrl *pctl = dev_get_drvdata(chip->parent);
>> + struct stm32_gpio_bank *bank = gpiochip_get_data(chip);
>> + unsigned int irq;
>> +
>> + regmap_field_write(pctl->irqmux[offset], bank->range.id);
>
> No. You must implement the irqchip and GPIO controllers to
> be orthogonal, doing things like this creates a semantic that
> assumes .to_irq() is always called before using the IRQ and
> that is not guaranteed at all. A consumer may very well
> use an interrupt right off the irqchip without this being called
> first. All this function should do is translate a number. No
> other semantics.
>
> This needs to be done from the irqchip (sorry).
Actually, the register written here is not part of the irqchip.
It is a system config register that is only used when using a GPIO as
external interrupt.
Its aim is to mux the GPIO bank on a line.
For example on line 1, it can be muxed to select either gpioa1,
gpiob1, gpioc1, ...
How could I do it in the irqchip that has no gpio knowledge?
In case the consumer uses an interrupt right off the irqchip, it
should not access this register.
For example the RTC wakeup lines, where we will reference directly the irqchip.
That said, do you still believe the implementation is wrong? Do I miss
something?
Best regards,
Maxime
Re: [PATCH v2 2/9] drivers: irqchip: Add STM32 external interrupts support
Hi Linus,
Sorry for the late reply, I was off last week.
2016-04-08 11:38 GMT+02:00 Linus Walleij :
> On Thu, Mar 31, 2016 at 5:09 PM, Maxime Coquelin
> wrote:
>
>> +static void stm32_irq_handler(struct irq_desc *desc)
>> +{
>> + struct irq_domain *domain = irq_desc_get_handler_data(desc);
>> + struct irq_chip_generic *gc = domain->gc->gc[0];
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>> + unsigned long pending;
>> + int n;
>> +
>> + chained_irq_enter(chip, desc);
>> +
>> + pending = irq_reg_readl(gc, EXTI_PR);
>> + for_each_set_bit(n, &pending, BITS_PER_LONG) {
>> + generic_handle_irq(irq_find_mapping(domain, n));
>> + }
>> +
>> + chained_irq_exit(chip, desc);
>> +}
>
> Is this one of those cases where you should re-read the status register
> on every iteration, so as to avoid exiting and immediately re-entering
> the irq handler?
>
> C.g irq-vic.c:
>
> static int handle_one_vic(struct vic_device *vic, struct pt_regs *regs)
> {
> u32 stat, irq;
> int handled = 0;
>
> while ((stat = readl_relaxed(vic->base + VIC_IRQ_STATUS))) {
> irq = ffs(stat) - 1;
> handle_domain_irq(vic->domain, irq, regs);
> handled = 1;
> }
>
> return handled;
> }
Indeed, it would be better doing it like this.
Do you think I could even do this with two nested loops to reduce the
number of reg accesses?
It would look like this (just compiled, not tested):
static void stm32_irq_handler(struct irq_desc *desc)
{
struct irq_domain *domain = irq_desc_get_handler_data(desc);
struct irq_chip_generic *gc = domain->gc->gc[0];
struct irq_chip *chip = irq_desc_get_chip(desc);
unsigned long pending;
int n;
chained_irq_enter(chip, desc);
while ((pending = irq_reg_readl(gc, EXTI_PR))) {
for_each_set_bit(n, &pending, BITS_PER_LONG) {
generic_handle_irq(irq_find_mapping(domain, n));
}
}
chained_irq_exit(chip, desc);
}
Thanks,
Maxime
Re: [PATCH v2 1/9] Documentation: dt-bindings: Document STM32 EXTI controller bindings
Hi Rob, 2016-04-04 7:15 GMT+02:00 Rob Herring : > On Thu, Mar 31, 2016 at 05:09:31PM +0200, Maxime Coquelin wrote: >> Signed-off-by: Maxime Coquelin >> --- >> .../bindings/interrupt-controller/st,stm32-exti.txt | 20 >> >> 1 file changed, 20 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt >> >> diff --git >> a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt >> b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt >> new file mode 100644 >> index ..6e7703d4ff5b >> --- /dev/null >> +++ >> b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt >> @@ -0,0 +1,20 @@ >> +STM32 External Interrupt Controller >> + >> +Required properties: >> + >> +- compatible: Should be "st,stm32-exti" >> +- reg: Specifies base physical address and size of the registers >> +- interrupt-controller: Indentifies the node as an interrupt controller >> +- #interrupt-cells: Specifies the number of cells to encode an interrupt >> + specifier, shall be 2 >> +- interrupts: interrupts references to primary interrupt controller > > Need to define how many and what is the order? The exti driver uses of_irq_count() to count the number of interrupts. The order doesn't matter in my implementation, I will come back on this point below. > > Are these 1:1 mapping? You could use interrupt-map here to define the > mapping. No, this is not 1:1 mapping. This is the mapping for STM32F429 (With 'n' managed by a mux in Syscfg (from GPIOA to GPIOI)): EXTI0 (GPIOn0) : NVIC 6 EXTI1 (GPIOn1) : NVIC 7 EXTI2 (GPIOn2) : NVIC 8 EXTI3 (GPIOn3) : NVIC 9 EXTI4 (GPIOn4) : NVIC 10 EXTI5 (GPIOn5) : NVIC 23 EXTI6 (GPIOn6) : NVIC 23 EXTI7 (GPIOn7) : NVIC 23 EXTI8 (GPIOn8) : NVIC 23 EXTI9 (GPIOn9) : NVIC 23 EXTI10 (GPIOn10) : NVIC 40 EXTI11 (GPIOn11) : NVIC 40 EXTI12 (GPIOn12) : NVIC 40 EXTI13 (GPIOn13) : NVIC 40 EXTI14 (GPIOn14) : NVIC 40 EXTI15 (GPIOn15) : NVIC 40 EXTI16 (PVD) : NVIC 1 EXTI17 (RTC Alarm) : NVIC 41 EXTI18 (USB OTG FS Wakeup) : NVIC 42 EXTI19 (Ethernet Wakeup) : NVIC 62 EXTI20 (USB OTG HS Wakeup) : NVIC 76 EXTI21 (RTC Tamper) : NVIC 2 EXTI22 (RTC Wakeup) : NVIC 3 Ideally, we should define a kind of mapping in the DT node. However, from what I understand 'interrupt-map' is not intended to be used in an interrupt controller (it would not even be parsed in of_irq_parse_raw()). Any ideas? Thanks for the review, Maxime
[PATCH v2 4/9] ARM: dts: Add EXTI controller node to stm32f429
Signed-off-by: Maxime Coquelin
---
arch/arm/boot/dts/stm32f429.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 35df462559ca..1a189d44ad38 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -176,6 +176,14 @@
reg = <0x40013800 0x400>;
};
+ exti: interrupt-controller@40013c00 {
+ compatible = "st,stm32-exti";
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ reg = <0x40013C00 0x400>;
+ interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>,
<23>, <40>, <41>, <42>, <62>, <76>;
+ };
+
pin-controller {
#address-cells = <1>;
#size-cells = <1>;
--
1.9.1
[PATCH v2 3/9] ARM: STM32: Select external interrupts controller
Signed-off-by: Maxime Coquelin --- arch/arm/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index cdfa6c2b7626..13b230a731eb 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -884,6 +884,7 @@ config ARCH_STM32 select CLKSRC_STM32 select PINCTRL select RESET_CONTROLLER + select STM32_EXTI help Support for STMicroelectronics STM32 processors. -- 1.9.1
[PATCH v2 8/9] ARM: dts: Declare push button as GPIO key on stm32f429 Disco board
Signed-off-by: Maxime Coquelin
---
arch/arm/boot/dts/stm32429i-eval.dts | 18 ++
arch/arm/boot/dts/stm32f429-disco.dts | 13 +
2 files changed, 31 insertions(+)
diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
b/arch/arm/boot/dts/stm32429i-eval.dts
index 6bfc5959dac3..0fd78e4e8a45 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -47,6 +47,7 @@
/dts-v1/;
#include "stm32f429.dtsi"
+#include
/ {
model = "STMicroelectronics STM32429i-EVAL board";
@@ -82,6 +83,23 @@
};
};
+ gpio_keys {
+ compatible = "gpio-keys";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ autorepeat;
+ button@0 {
+ label = "Wake up";
+ linux,code = ;
+ gpios = <&gpioa 0 0>;
+ };
+ button@1 {
+ label = "Tamper";
+ linux,code = ;
+ gpios = <&gpioc 13 0>;
+ };
+ };
+
usbotg_hs_phy: usbphy {
#phy-cells = <0>;
compatible = "usb-nop-xceiv";
diff --git a/arch/arm/boot/dts/stm32f429-disco.dts
b/arch/arm/boot/dts/stm32f429-disco.dts
index 01408073dd53..7d0415e80668 100644
--- a/arch/arm/boot/dts/stm32f429-disco.dts
+++ b/arch/arm/boot/dts/stm32f429-disco.dts
@@ -47,6 +47,7 @@
/dts-v1/;
#include "stm32f429.dtsi"
+#include
/ {
model = "STMicroelectronics STM32F429i-DISCO board";
@@ -75,6 +76,18 @@
linux,default-trigger = "heartbeat";
};
};
+
+ gpio_keys {
+ compatible = "gpio-keys";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ autorepeat;
+ button@0 {
+ label = "User";
+ linux,code = ;
+ gpios = <&gpioa 0 0>;
+ };
+ };
};
&clk_hse {
--
1.9.1
[PATCH v2 1/9] Documentation: dt-bindings: Document STM32 EXTI controller bindings
Signed-off-by: Maxime Coquelin
---
.../bindings/interrupt-controller/st,stm32-exti.txt | 20
1 file changed, 20 insertions(+)
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt
b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt
new file mode 100644
index ..6e7703d4ff5b
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt
@@ -0,0 +1,20 @@
+STM32 External Interrupt Controller
+
+Required properties:
+
+- compatible: Should be "st,stm32-exti"
+- reg: Specifies base physical address and size of the registers
+- interrupt-controller: Indentifies the node as an interrupt controller
+- #interrupt-cells: Specifies the number of cells to encode an interrupt
+ specifier, shall be 2
+- interrupts: interrupts references to primary interrupt controller
+
+Example:
+
+exti: interrupt-controller@40013c00 {
+ compatible = "st,stm32-exti";
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ reg = <0x40013C00 0x400>;
+ interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, <23>, <40>, <41>,
<42>, <62>, <76>;
+};
--
1.9.1
[PATCH v2 6/9] pinctrl: Add IRQ support to STM32 gpios
This patch adds IRQ support to STM32 gpios.
The EXTI controller has 16 lines dedicated to GPIOs.
EXTI line n can be connected to only line n of one of the GPIO ports, for
example EXTI0 can be connected to either PA0, or PB0, or PC0...
This port selection is done by specifying the port number into System
Config registers.
Signed-off-by: Maxime Coquelin
---
drivers/pinctrl/stm32/Kconfig | 1 +
drivers/pinctrl/stm32/pinctrl-stm32.c | 68 +++
2 files changed, 69 insertions(+)
diff --git a/drivers/pinctrl/stm32/Kconfig b/drivers/pinctrl/stm32/Kconfig
index 0f28841b2332..b5cac5bfd0cd 100644
--- a/drivers/pinctrl/stm32/Kconfig
+++ b/drivers/pinctrl/stm32/Kconfig
@@ -6,6 +6,7 @@ config PINCTRL_STM32
select PINMUX
select GENERIC_PINCONF
select GPIOLIB
+ select MFD_SYSCON
config PINCTRL_STM32F429
bool "STMicroelectronics STM32F429 pin control" if COMPILE_TEST &&
!MACH_STM32F429
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c
b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 8deb566ed4cd..f2fa717894dc 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -8,6 +8,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -20,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
@@ -77,6 +80,9 @@ struct stm32_pinctrl {
struct stm32_gpio_bank *banks;
unsigned nbanks;
const struct stm32_pinctrl_match_data *match_data;
+ struct irq_domain *domain;
+ struct regmap *regmap;
+ struct regmap_field *irqmux[STM32_GPIO_PINS_PER_BANK];
};
static inline int stm32_gpio_pin(int gpio)
@@ -174,6 +180,22 @@ static int stm32_gpio_direction_output(struct gpio_chip
*chip,
return 0;
}
+
+static int stm32_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ struct stm32_pinctrl *pctl = dev_get_drvdata(chip->parent);
+ struct stm32_gpio_bank *bank = gpiochip_get_data(chip);
+ unsigned int irq;
+
+ regmap_field_write(pctl->irqmux[offset], bank->range.id);
+
+ irq = irq_create_mapping(pctl->domain, offset);
+ if (!irq)
+ return -ENXIO;
+
+ return irq;
+}
+
static struct gpio_chip stm32_gpio_template = {
.request= stm32_gpio_request,
.free = stm32_gpio_free,
@@ -181,6 +203,7 @@ static struct gpio_chip stm32_gpio_template = {
.set= stm32_gpio_set,
.direction_input= stm32_gpio_direction_input,
.direction_output = stm32_gpio_direction_output,
+ .to_irq = stm32_gpio_to_irq,
};
/* Pinctrl functions */
@@ -704,6 +727,47 @@ static int stm32_gpiolib_register_bank(struct
stm32_pinctrl *pctl,
return 0;
}
+static int stm32_pctrl_dt_setup_irq(struct platform_device *pdev,
+ struct stm32_pinctrl *pctl)
+{
+ struct device_node *np = pdev->dev.of_node, *parent;
+ struct device *dev = &pdev->dev;
+ struct regmap *rm;
+ int offset, ret, i;
+
+ parent = of_irq_find_parent(np);
+ if (!parent)
+ return -ENXIO;
+
+ pctl->domain = irq_find_host(parent);
+ if (!pctl->domain)
+ return -ENXIO;
+
+ pctl->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
+ if (IS_ERR(pctl->regmap))
+ return PTR_ERR(pctl->regmap);
+
+ rm = pctl->regmap;
+
+ ret = of_property_read_u32_index(np, "st,syscfg", 1, &offset);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < STM32_GPIO_PINS_PER_BANK; i++) {
+ struct reg_field mux;
+
+ mux.reg = offset + (i / 4) * 4;
+ mux.lsb = (i % 4) * 4;
+ mux.msb = mux.lsb + 3;
+
+ pctl->irqmux[i] = devm_regmap_field_alloc(dev, rm, mux);
+ if (IS_ERR(pctl->irqmux[i]))
+ return PTR_ERR(pctl->irqmux[i]);
+ }
+
+ return 0;
+}
+
static int stm32_pctrl_build_state(struct platform_device *pdev)
{
struct stm32_pinctrl *pctl = platform_get_drvdata(pdev);
@@ -796,6 +860,10 @@ int stm32_pctl_probe(struct platform_device *pdev)
}
}
+ ret = stm32_pctrl_dt_setup_irq(pdev, pctl);
+ if (ret)
+ return ret;
+
pins = devm_kcalloc(&pdev->dev, pctl->match_data->npins, sizeof(*pins),
GFP_KERNEL);
if (!pins)
--
1.9.1
[PATCH v2 7/9] ARM: dts: Add GPIO irq support to STM2F429
Signed-off-by: Maxime Coquelin
---
arch/arm/boot/dts/stm32f429.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 1a189d44ad38..68247625a8d5 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -189,6 +189,8 @@
#size-cells = <1>;
compatible = "st,stm32f429-pinctrl";
ranges = <0 0x4002 0x3000>;
+ interrupt-parent = <&exti>;
+ st,syscfg = <&syscfg 0x8>;
pins-are-numbered;
gpioa: gpio@4002 {
--
1.9.1
[PATCH v2 9/9] ARM: config: Enable GPIO Key driver in stm32_defconfig
Signed-off-by: Maxime Coquelin --- arch/arm/configs/stm32_defconfig | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index 1e5ec2a0e4cf..e7b56d4f1798 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -38,7 +38,11 @@ CONFIG_DEVTMPFS_MOUNT=y # CONFIG_FW_LOADER is not set # CONFIG_BLK_DEV is not set CONFIG_EEPROM_93CX6=y -# CONFIG_INPUT is not set +# CONFIG_INPUT_LEDS is not set +# CONFIG_INPUT_MOUSEDEV is not set +# CONFIG_KEYBOARD_ATKBD is not set +CONFIG_KEYBOARD_GPIO=y +# CONFIG_INPUT_MOUSE is not set # CONFIG_SERIO is not set # CONFIG_VT is not set # CONFIG_UNIX98_PTYS is not set -- 1.9.1
[PATCH v2 5/9] Documentation: dt-bindings: Add IRQ related properties of STM32 pinctrl
Signed-off-by: Maxime Coquelin --- Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt index 7b4800cc251e..dd95becba966 100644 --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt @@ -13,6 +13,9 @@ Required properies: - #size-cells : The value of this property must be 1 - ranges : defines mapping between pin controller node (parent) to gpio-bank node (children). + - interrupt-parent: phandle of the interrupt parent to which the external + GPIO interrupts are forwarded to. + - st,syscfg: phandle of the syscfg node used for IRQ mux selection. - pins-are-numbered: Specify the subnodes are using numbered pinmux to specify pins. -- 1.9.1
[PATCH v2 0/9] Add STM32 EXTI interrupt controller support
Hi, This v2 is only a rebase on top of v4.6-rc1 and also changes a variable name. The series adds support to EXTI interrupt controller and GPIO IRQ support in STM32 pinctrl driver. The STM32 external interrupt controller consists of edge detectors that generate interrupts requests or wake-up events. Each line can be independently configured as interrupt or wake-up source, and triggers either on rising, fallin or both edges. Each line can also be masked independently. Regards, Maxime Changes since v1: - - Rebased on top of v4.6-rc1 - Change variable name from virq to irq (Linus W.) Maxime Coquelin (9): Documentation: dt-bindings: Document STM32 EXTI controller bindings drivers: irqchip: Add STM32 external interrupts support ARM: STM32: Select external interrupts controller ARM: dts: Add EXTI controller node to stm32f429 Documentation: dt-bindings: Add IRQ related properties of STM32 pinctrl pinctrl: Add IRQ support to STM32 gpios ARM: dts: Add GPIO irq support to STM2F429 ARM: dts: Declare push button as GPIO key on stm32f429 Disco board ARM: config: Enable GPIO Key driver in stm32_defconfig .../interrupt-controller/st,stm32-exti.txt | 20 +++ .../bindings/pinctrl/st,stm32-pinctrl.txt | 3 + arch/arm/Kconfig | 1 + arch/arm/boot/dts/stm32429i-eval.dts | 18 +++ arch/arm/boot/dts/stm32f429-disco.dts | 13 ++ arch/arm/boot/dts/stm32f429.dtsi | 10 ++ arch/arm/configs/stm32_defconfig | 6 +- drivers/irqchip/Kconfig| 4 + drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-stm32-exti.c | 169 + drivers/pinctrl/stm32/Kconfig | 1 + drivers/pinctrl/stm32/pinctrl-stm32.c | 68 + 12 files changed, 313 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/st,stm32-exti.txt create mode 100644 drivers/irqchip/irq-stm32-exti.c -- 1.9.1
[PATCH v2 2/9] drivers: irqchip: Add STM32 external interrupts support
The STM32 external interrupt controller consists of edge detectors that
generate interrupts requests or wake-up events.
Each line can be independently configured as interrupt or wake-up source,
and triggers either on rising, fallin or both edges. Each line can also
be masked independently.
Signed-off-by: Maxime Coquelin
---
drivers/irqchip/Kconfig | 4 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-stm32-exti.c | 169 +++
3 files changed, 174 insertions(+)
create mode 100644 drivers/irqchip/irq-stm32-exti.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 3e124793e224..149a30e0eec0 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -244,3 +244,7 @@ config IRQ_MXS
config MVEBU_ODMI
bool
select GENERIC_MSI_IRQ_DOMAIN
+
+config STM32_EXTI
+ bool
+ select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b03cfcbbac6b..2caeae000307 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -65,3 +65,4 @@ obj-$(CONFIG_INGENIC_IRQ) += irq-ingenic.o
obj-$(CONFIG_IMX_GPCV2)+= irq-imx-gpcv2.o
obj-$(CONFIG_PIC32_EVIC) += irq-pic32-evic.o
obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o
+obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
new file mode 100644
index ..02bfa807db6c
--- /dev/null
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -0,0 +1,169 @@
+/*
+ * Copyright (C) Maxime Coquelin 2015
+ * Author: Maxime Coquelin
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define EXTI_IMR 0x0
+#define EXTI_EMR 0x4
+#define EXTI_RTSR 0x8
+#define EXTI_FTSR 0xc
+#define EXTI_SWIER 0x10
+#define EXTI_PR0x14
+
+static void stm32_irq_handler(struct irq_desc *desc)
+{
+ struct irq_domain *domain = irq_desc_get_handler_data(desc);
+ struct irq_chip_generic *gc = domain->gc->gc[0];
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ unsigned long pending;
+ int n;
+
+ chained_irq_enter(chip, desc);
+
+ pending = irq_reg_readl(gc, EXTI_PR);
+ for_each_set_bit(n, &pending, BITS_PER_LONG) {
+ generic_handle_irq(irq_find_mapping(domain, n));
+ }
+
+ chained_irq_exit(chip, desc);
+}
+
+static int stm32_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+ u32 rtsr, ftsr;
+ int pin = data->hwirq;
+
+ irq_gc_lock(gc);
+
+ rtsr = irq_reg_readl(gc, EXTI_RTSR);
+ ftsr = irq_reg_readl(gc, EXTI_FTSR);
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_RISING:
+ rtsr |= BIT(pin);
+ ftsr &= ~BIT(pin);
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ rtsr &= ~BIT(pin);
+ ftsr |= BIT(pin);
+ break;
+ case IRQ_TYPE_EDGE_BOTH:
+ rtsr |= BIT(pin);
+ ftsr |= BIT(pin);
+ break;
+ default:
+ irq_gc_unlock(gc);
+ return -EINVAL;
+ }
+
+ irq_reg_writel(gc, rtsr, EXTI_RTSR);
+ irq_reg_writel(gc, ftsr, EXTI_FTSR);
+
+ irq_gc_unlock(gc);
+
+ return 0;
+}
+
+static int stm32_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+ int pin = data->hwirq;
+ u32 emr;
+
+ irq_gc_lock(gc);
+
+ emr = irq_reg_readl(gc, EXTI_EMR);
+ if (on)
+ emr |= BIT(pin);
+ else
+ emr &= ~BIT(pin);
+ irq_reg_writel(gc, emr, EXTI_EMR);
+
+ irq_gc_unlock(gc);
+
+ return 0;
+}
+
+static int __init stm32_exti_init(struct device_node *node,
+ struct device_node *parent)
+{
+ int nr_irqs, nr_exti, ret, i;
+ unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
+ struct irq_domain *domain;
+ struct irq_chip_generic *gc;
+ void *base;
+
+ base = of_iomap(node, 0);
+ if (!base) {
+ pr_err("%s: Unable to map registers\n", node->full_name);
+ return -ENOMEM;
+ }
+
+ /* Determine number of irqs supported */
+ writel_relaxed(~0UL, base + EXTI_RTSR);
+ nr_exti = fls(readl_relaxed(base + EXTI_RTSR));
+ writel_relaxed(0, base + EXTI_RTSR);
+
+ pr_info("%s: %d External IRQs detected\n", node->full_name, nr_exti);
+
+ domain = irq_domain_add_linear(node, nr_exti,
+ &irq_generic_chip_ops, NULL);
+ if
Re: [PATCH] MAINTAINERS: Add the stlinux kernel mailing list for the STi drm driver.
On 03/17/2016 04:07 PM, Peter Griffin wrote: Hi Lee, On Thu, 17 Mar 2016, Lee Jones wrote: On Thu, 17 Mar 2016, Peter Griffin wrote: Most drivers for STi are under the ARM/STI Architecture section which includes this mailing list. Signed-off-by: Peter Griffin --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 6ee06ea..c541a89 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3814,6 +3814,7 @@ DRM DRIVERS FOR STI M:Benjamin Gaignard M:Vincent Abriou L:dri-de...@lists.freedesktop.org +L: ker...@stlinux.com T:git http://git.linaro.org/people/benjamin.gaignard/kernel.git S:Maintained F:drivers/gpu/drm/sti Doesn't it make more sense to add drivers/gpu/drm/sti/ to the ARM/STI ARCHITECTURE list of supported files? I assumed Vincent and Benjamin deliberately added a new entry because they want to be maintainers of this driver (which I think probably makes sense given that the display IP is quite complicated). I agree with Peter, let's just add kernel@stlinux entry in the DRM STi entry. Acked-by: Maxime Coquelin Regards, Maxime
Re: [PATCH v2 0/4] Add Ethernet support on STM32F429
Hi Alex, On 02/23/2016 04:10 PM, Alexandre TORGUE wrote: STM32F429 Chip embeds a Synopsys 3.50a MAC IP. This series: -enhance current stmmac driver to control it (code already available) and adds basic glue for STM32F429 chip. -Enable basic Net config in kernel. Note that DT patches are not present because STM32 pinctrl code is not yet avalaible. Changes since v1: -Fix Kbuild issue in Kconfig. -Remove init/exit callbacks. Suspend/Resume and remove driver is no more driven in stmmac_pltfr but directly in dwmac-stm32 glue driver. -Take into account Joachim review. Regards. Alexandre. Alexandre TORGUE (4): net: ethernet: dwmac: add Ethernet glue logic for stm32 chip Documentation: Bindings: Add STM32 DWMAC glue net: ethernet: stmmac: add support of Synopsys 3.50a MAC IP ARM: STM32: Enable Ethernet in stm32_defconfig .../devicetree/bindings/net/stm32-dwmac.txt| 41 arch/arm/configs/stm32_defconfig | 9 + drivers/net/ethernet/stmicro/stmmac/Kconfig| 12 ++ drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c | 208 + .../net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 + 6 files changed, 272 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/stm32-dwmac.txt create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c You can add my: Tested-by: Maxime Coquelin Thanks! Maxime
Re: [PATCH v3 4/4] ARM: STM32: Enable Ethernet in stm32_defconfig
Hi Alex, Arnd, On 02/26/2016 11:51 AM, Alexandre TORGUE wrote: Enable basic Ethernet support (IPV4) for stm32 defconfig. Signed-off-by: Alexandre TORGUE diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index ec52505..8b8abe0 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -33,11 +33,20 @@ CONFIG_XIP_PHYS_ADDR=0x08008000 CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set +CONFIG_NET=y +CONFIG_INET=y +# CONFIG_INET_XFRM_MODE_TRANSPORT is not set +# CONFIG_INET_XFRM_MODE_TUNNEL is not set +# CONFIG_INET_XFRM_MODE_BEET is not set +# CONFIG_IPV6 is not set CONFIG_DEVTMPFS=y CONFIG_DEVTMPFS_MOUNT=y # CONFIG_FW_LOADER is not set # CONFIG_BLK_DEV is not set CONFIG_EEPROM_93CX6=y +CONFIG_NETDEVICES=y +CONFIG_STMMAC_ETH=y +# CONFIG_WLAN is not set # CONFIG_INPUT is not set # CONFIG_SERIO is not set # CONFIG_VT is not set The patch looks good, but I'm not sure I should apply it. Indeed, not all the stm32 boards have Ethernet interface, and stm32 internal flash size is rather limited (2MB). Note that your glue layer can be compiled with COMPILE_TEST, so its build will be covered anyway. Arnd, do you think we could use Kconfig fragments to enable features like USB, Ethernet, MTD, etc...? Maybe we could have a subdirectory in arch/arm/configs where these stm32 specifics fragments could leave? For example, we could have: arch/arm/configs/stm32/eth.config arch/arm/configs/stm32/usb.config For configuring the Discovery board we would use: make stm32_defconfig stm32/usb.config For the Eval board, we cwould use: make stm32_defconfig eth.config It would make easy to be able to boot every boards without multiplying the defconfigs. Regards, Maxime
Re: [PATCH 2/3] ARM: dts: stm32f429: Add Ethernet support
Hi Alex,
I have made a handful of changes on your patch, let me know if this
is ok for you.
If ok, it will be part of the PR I'll send tomorrow.
On 02/29/2016 05:29 PM, Alexandre TORGUE wrote:
Add Ethernet support (Synopsys MAC IP 3.50a) on stm32f429 SOC.
Signed-off-by: Alexandre TORGUE
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index bb7a736..af0367c 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -283,6 +283,26 @@
bias-disable;
};
};
+
+ ethernet0_mii: mii@0 {
+ mii {
+ slew-rate = <2>;
I moved slew-rate property below the pinmux one for consistency with
other pin configs in the file.
+ pinmux =
,
+
,
+
,
+
,
+
,
+
,
+,
+,
+
,
+
,
+
,
+
,
+
,
+
;
+ };
+ };
};
rcc: rcc@40023810 {
@@ -323,6 +343,21 @@
st,mem2mem;
};
+ ethernet0: dwmac@40028000 {
+ compatible = "st,stm32-dwmac", "snps,dwmac-3.50a";
+ status = "disabled";
I moved status property at the end of the node for consistency
+ reg = <0x40028000 0x8000>;
+ reg-names = "stmmaceth";
+ interrupts = <0 61 0>, <0 62 0>;
#interrupt-cells is set to 1 in the nvic node, meaning that a single
cell is expected here:
interrupts = <61>, <62>;
+ interrupt-names = "macirq", "eth_wake_irq";
+ clock-names = "stmmaceth", "tx-clk", "rx-clk";
+ clocks = <&rcc 0 25>, <&rcc 0 26>, <&rcc 0 27>;
+ st,syscon = <&syscfg 0x4>;
+ snps,pbl = <8>;
+ snps,mixed-burst;
+ dma-ranges;
+ };
+
rng: rng@50060800 {
compatible = "st,stm32-rng";
reg = <0x50060800 0x400>;
Regards,
Maxime
Re: [PATCH 5/8] drivers/pinctrl: make stm32/pinctrl-stm32f429.c explicitly non-modular
2016-02-29 21:48 GMT+01:00 Paul Gortmaker : > The Kconfig currently controlling compilation of this code is: > > drivers/pinctrl/stm32/Kconfig:config PINCTRL_STM32F429 > drivers/pinctrl/stm32/Kconfig: bool "STMicroelectronics STM32F429 pin > control" if COMPILE_TEST && !MACH_STM32F429 > > ...meaning that it currently is not being built as a module by anyone. > > Lets remove the modular code that is essentially orphaned, so that > when reading the driver there is no doubt it is builtin-only. > > Since module_init translates to device_initcall in the non-modular > case, the init ordering remains unchanged with this commit. > > Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code. > > We also delete the MODULE_LICENSE tag etc. since all that information > is already contained at the top of the file in the comments. > > Cc: Linus Walleij > Cc: Maxime Coquelin > Cc: Patrice Chotard > Cc: linux-g...@vger.kernel.org > Cc: linux-arm-ker...@lists.infradead.org > Signed-off-by: Paul Gortmaker > --- > drivers/pinctrl/stm32/pinctrl-stm32f429.c | 9 + > 1 file changed, 1 insertion(+), 8 deletions(-) Thanks for the patch: Acked-by: Maxime Coquelin Regards, Maxime
[PATCH] ARM: dts: stm32429i-eval: Add USB HS host mode support
This patch adds USB HS support in host mode only.
This port supports OTG mode, but the device more is not working
properly as of now.
Once the device mode fixed, the node will be updated to support OTG.
Signed-off-by: Maxime Coquelin
---
arch/arm/boot/dts/stm32429i-eval.dts | 16
arch/arm/boot/dts/stm32f429.dtsi | 30 ++
2 files changed, 46 insertions(+)
diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
b/arch/arm/boot/dts/stm32429i-eval.dts
index 1ae57fa..76a10d3 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -81,6 +81,13 @@
gpios = <&gpiog 12 1>;
};
};
+
+ usbotg_hs_phy: usbphy {
+ #phy-cells = <0>;
+ compatible = "usb-nop-xceiv";
+ clocks = <&rcc 0 30>;
+ clock-names = "main_clk";
+ };
};
&clk_hse {
@@ -92,3 +99,12 @@
pinctrl-names = "default";
status = "okay";
};
+
+&usbotg_hs {
+ dr_mode = "host";
+ phys = <&usbotg_hs_phy>;
+ phy-names = "usb2-phy";
+ pinctrl-0 = <&usbotg_hs_pins_a>;
+ pinctrl-names = "default";
+ status = "okay";
+};
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 598362e..ee82756 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -278,6 +278,26 @@
bias-disable;
};
};
+
+ usbotg_hs_pins_a: usbotg_hs@0 {
+ pins {
+ pinmux =
,
+
,
+
,
+
,
+
,
+
,
+
,
+
,
+
,
+
,
+
,
+
;
+ bias-disable;
+ drive-push-pull;
+ slew-rate = <2>;
+ };
+ };
};
rcc: rcc@40023810 {
@@ -318,6 +338,16 @@
st,mem2mem;
};
+ usbotg_hs: usb@4004 {
+ compatible = "snps,dwc2";
+ dma-ranges;
+ reg = <0x4004 0x4>;
+ interrupts = <77>;
+ clocks = <&rcc 0 29>;
+ clock-names = "otg";
+ status = "disabled";
+ };
+
rng: rng@50060800 {
compatible = "st,stm32-rng";
reg = <0x50060800 0x400>;
--
1.9.1
Re: [PATCH 27/50] pinctrl: stm32: Use devm_pinctrl_register() for pinctrl registration
2016-02-24 14:15 GMT+01:00 Laxman Dewangan : > Use devm_pinctrl_register() for pin control registration. > > Signed-off-by: Laxman Dewangan > Cc: Maxime Coquelin > Cc: Patrice Chotard > --- > drivers/pinctrl/stm32/pinctrl-stm32.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) Acked-by: Maxime Coquelin Thanks! Maxime
Re: [PATCH 45/50] pinctrl: st: Use devm_pinctrl_register() for pinctrl registration
On 02/24/2016 02:16 PM, Laxman Dewangan wrote: Use devm_pinctrl_register() for pin control registration. Signed-off-by: Laxman Dewangan Cc: Srinivas Kandagatla Cc: Maxime Coquelin Cc: Patrice Chotard Cc: ker...@stlinux.com --- drivers/pinctrl/pinctrl-st.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Maxime Coquelin Thanks! Maxime
[PATCH] ARM: dts: stm32f429: Fix clocks referenced by GPIO banks
All the clocks referenced by the GPIO banks were not the good ones. Reported-by: Bruno Herrera Signed-off-by: Maxime Coquelin --- arch/arm/boot/dts/stm32f429.dtsi | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi index 35b2ab1..598362e 100644 --- a/arch/arm/boot/dts/stm32f429.dtsi +++ b/arch/arm/boot/dts/stm32f429.dtsi @@ -182,7 +182,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x0 0x400>; - clocks = <&rcc 0 256>; + clocks = <&rcc 0 0>; st,bank-name = "GPIOA"; }; @@ -190,7 +190,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x400 0x400>; - clocks = <&rcc 0 257>; + clocks = <&rcc 0 1>; st,bank-name = "GPIOB"; }; @@ -198,7 +198,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x800 0x400>; - clocks = <&rcc 0 258>; + clocks = <&rcc 0 2>; st,bank-name = "GPIOC"; }; @@ -206,7 +206,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0xc00 0x400>; - clocks = <&rcc 0 259>; + clocks = <&rcc 0 3>; st,bank-name = "GPIOD"; }; @@ -214,7 +214,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x1000 0x400>; - clocks = <&rcc 0 260>; + clocks = <&rcc 0 4>; st,bank-name = "GPIOE"; }; @@ -222,7 +222,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x1400 0x400>; - clocks = <&rcc 0 261>; + clocks = <&rcc 0 5>; st,bank-name = "GPIOF"; }; @@ -230,7 +230,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x1800 0x400>; - clocks = <&rcc 0 262>; + clocks = <&rcc 0 6>; st,bank-name = "GPIOG"; }; @@ -238,7 +238,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x1c00 0x400>; - clocks = <&rcc 0 263>; + clocks = <&rcc 0 7>; st,bank-name = "GPIOH"; }; @@ -246,7 +246,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x2000 0x400>; - clocks = <&rcc 0 264>; + clocks = <&rcc 0 8>; st,bank-name = "GPIOI"; }; @@ -254,7 +254,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x2400 0x400>; - clocks = <&rcc 0 265>; + clocks = <&rcc 0 9>; st,bank-name = "GPIOJ"; }; @@ -262,7 +262,7 @@ gpio-controller; #gpio-cells = <2>; reg = <0x2800 0x400>; - clocks = <&rcc 0 266>; + clocks = <&rcc 0 10>; st,bank-name = "GPIOK"; }; -- 1.9.1
Re: [PATCH 0/3] ARM: stm32: Introduce support for STM32F469
Hi Lee, On 02/16/2016 03:16 PM, Lee Jones wrote: Simple set using the STM32F429 infrastructure as a foundation. There will be more changes to the DTS files and a requirement for added hierarchical layering as more differences become apparent. Lee Jones (3): ARM: stm32: Supply a DTS file for the STM32F469 Discovery board ARM: configs: Add new config fragment to change RAM start point ARM: stm32: Identify a new board - STM32F469 arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/stm32f469-disco.dts | 75 + arch/arm/configs/dram_0x.config | 1 + arch/arm/mach-stm32/board-dt.c | 1 + 4 files changed, 78 insertions(+) create mode 100644 arch/arm/boot/dts/stm32f469-disco.dts create mode 100644 arch/arm/configs/dram_0x.config Series applied to my tree for v4.6. Thanks! Maxime
Re: [PATCH v4 4/9] pinctrl: Add STM32 MCUs support
On 02/13/2016 03:48 PM, Linus Walleij wrote: On Mon, Feb 1, 2016 at 1:09 PM, Maxime Coquelin wrote: On 02/01/2016 11:39 AM, Maxime Coquelin wrote: 2016-02-01 11:21 GMT+01:00 Arnd Bergmann : drivers/pinctrl/stm32/pinctrl-stm32.c:26:47: fatal error: dt-bindings/pinctrl/pinctrl-stm32.h: No such file or directory Right, I missed to notify this dependency to Linus, sorry about that. This dependency is no more needed, so I could just move the macros to the driver side. Note that I also noticed a breakage at build time due to field renaming in gpio_chip structure (dev -> parent). I propose to rebase the series and move the defines into the driver. Is it ok for you? I have rebased the series, taking care to fix the mentioned problems. Linus, do you confirm you will revert STM32 pinctrl patches from your tree? I guess the other fixes removes the need to do this. Exactly, all should be fine now. Sorry for not staying on top of this issue, I was swamped with mail and had a hard time prioritizing :( No worries! Thanks, Maxime
[PATCH 0/2] pinctrl: stm32; Fix build issue in -next and make compile-testing possible
This two-patches series fixes two problems on stm32 pinctrl driver: 1 - As reported by Arnd, there is a missing dependency with DT Bindings headers on Linus' pinctrl/devel branch. This dependency is in fact not needed, so the first patch removes it. 2 - The driver could be selected for compile testing via menuconfig, but the Makefile only selected stm32 driver directory if ARCH_STM32 was selected. The second patch selects the stm32 driver if PINCTRL_STM32 is selected. Regards, Maxime Maxime Coquelin (2): pinctrl: stm32: Remove dependency with DT bindings header files pinctrl: stm32: Fix compile testing selection drivers/pinctrl/Makefile | 2 +- drivers/pinctrl/stm32/pinctrl-stm32.c | 2 -- drivers/pinctrl/stm32/pinctrl-stm32.h | 8 3 files changed, 9 insertions(+), 3 deletions(-) -- 1.9.1
[PATCH 2/2] pinctrl: stm32: Fix compile testing selection
While selecting the driver for compile testing seemed possible, the driver was not compiled because the driver directory was only added if ARCH_STM32 was selected. This patch now makes the pinctrl Makefile to add stm32 directory if PINCTRL_STM32 is selected. Signed-off-by: Maxime Coquelin --- drivers/pinctrl/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index 8adc235..7ea62ef 100644 --- a/drivers/pinctrl/Makefile +++ b/drivers/pinctrl/Makefile @@ -46,7 +46,7 @@ obj-$(CONFIG_ARCH_QCOM) += qcom/ obj-$(CONFIG_PINCTRL_SAMSUNG) += samsung/ obj-$(CONFIG_PINCTRL_SH_PFC) += sh-pfc/ obj-$(CONFIG_PINCTRL_SPEAR)+= spear/ -obj-$(CONFIG_ARCH_STM32) += stm32/ +obj-$(CONFIG_PINCTRL_STM32)+= stm32/ obj-$(CONFIG_PINCTRL_SUNXI)+= sunxi/ obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/ obj-$(CONFIG_ARCH_VT8500) += vt8500/ -- 1.9.1
[PATCH 1/2] pinctrl: stm32: Remove dependency with DT bindings header files
Some macros where defined in DT bindings headers, whereas only used
in the driver.
This patch moves these macros to the driver side.
Signed-off-by: Maxime Coquelin
---
drivers/pinctrl/stm32/pinctrl-stm32.c | 2 --
drivers/pinctrl/stm32/pinctrl-stm32.h | 8
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c
b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 9a08222..8deb566 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -23,8 +23,6 @@
#include
#include
-#include
-
#include "../core.h"
#include "../pinconf.h"
#include "../pinctrl-utils.h"
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.h
b/drivers/pinctrl/stm32/pinctrl-stm32.h
index 1b7c369..35ebc94 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.h
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.h
@@ -9,6 +9,14 @@
#include
#include
+#define STM32_PIN_NO(x) ((x) << 8)
+#define STM32_GET_PIN_NO(x) ((x) >> 8)
+#define STM32_GET_PIN_FUNC(x) ((x) & 0xff)
+
+#define STM32_PIN_GPIO 0
+#define STM32_PIN_AF(x)((x) + 1)
+#define STM32_PIN_ANALOG (STM32_PIN_AF(15) + 1)
+
struct stm32_desc_function {
const char *name;
const unsigned char num;
--
1.9.1
Re: [PATCH v5 0/9] Add STM32 pinctrl/GPIO driver
2016-02-03 12:04 GMT+01:00 Maxime Coquelin : > Hi Linus, > > This is the fifth round of STM32 pinctrl series, which is rebased on top of > v4.5-rc1 and removes no more needed dependency between DT and driver. > > Also, it fixes the Makefile so that it is really built when COMPILE_TEST is > set and not ARCH_STM32. > > The STM32 family has 16 pins per GPIO bank, and the number of bank varies > depending on the model. > > Pins can be multiplexed either in GPIO mode, alternate function (up to 15 > functions per pin) or analog (for ADC/DAC). > > Changes since v4: > - > - Rebase on v4.5-rc1 > - Fix compilation breakage due to gpio_chip struct .dev field renaming > - Move the macros from includes/dt-bindings to STM32 pinctrl driver directory > to avoid uneeded dependency between DT and driver. > - Fix Makefile so that driver is built with COMPILE_TEST set Please discard this series, the changes above are being handled on top of v4. Regards, Maxime
Re: [PATCH] pinctrl: stm32: fix compile error and modernize
2016-02-05 23:49 GMT+01:00 Linus Walleij : > - Fix the dev->parent assignment compile error > - Use gpiochip_get_data() to get the data pointer for the > banks > > Cc: Maxime Coquelin > Cc: Patrice Chotard > Signed-off-by: Linus Walleij > --- > I don't even know how to compile test this, I hope it works, > Maxime can you verify? Thanks Linus, I just tested your patch with success: Tested-by: Maxime Coquelin About the compile testing, I have identified the problem last week, and sent a v5 of the series. I will discard this series, and will send you a patch that fixes compile testing, and also unneeded dependency with DT bindings header file. Regards, Maxime
[PATCH v5 3/9] includes: dt-bindings: Add STM32F429 pinctrl DT bindings
Acked-by: Patrice Chotard Acked-by: Linus Walleij Signed-off-by: Maxime Coquelin --- include/dt-bindings/pinctrl/stm32f429-pinfunc.h | 1239 +++ 1 file changed, 1239 insertions(+) create mode 100644 include/dt-bindings/pinctrl/stm32f429-pinfunc.h diff --git a/include/dt-bindings/pinctrl/stm32f429-pinfunc.h b/include/dt-bindings/pinctrl/stm32f429-pinfunc.h new file mode 100644 index 000..26f1879 --- /dev/null +++ b/include/dt-bindings/pinctrl/stm32f429-pinfunc.h @@ -0,0 +1,1239 @@ +#ifndef _DT_BINDINGS_STM32F429_PINFUNC_H +#define _DT_BINDINGS_STM32F429_PINFUNC_H + +#define STM32F429_PA0_FUNC_GPIO 0x0 +#define STM32F429_PA0_FUNC_TIM2_CH1_TIM2_ETR 0x2 +#define STM32F429_PA0_FUNC_TIM5_CH1 0x3 +#define STM32F429_PA0_FUNC_TIM8_ETR 0x4 +#define STM32F429_PA0_FUNC_USART2_CTS 0x8 +#define STM32F429_PA0_FUNC_UART4_TX 0x9 +#define STM32F429_PA0_FUNC_ETH_MII_CRS 0xc +#define STM32F429_PA0_FUNC_EVENTOUT 0x10 +#define STM32F429_PA0_FUNC_ANALOG 0x11 + +#define STM32F429_PA1_FUNC_GPIO 0x100 +#define STM32F429_PA1_FUNC_TIM2_CH2 0x102 +#define STM32F429_PA1_FUNC_TIM5_CH2 0x103 +#define STM32F429_PA1_FUNC_USART2_RTS 0x108 +#define STM32F429_PA1_FUNC_UART4_RX 0x109 +#define STM32F429_PA1_FUNC_ETH_MII_RX_CLK_ETH_RMII_REF_CLK 0x10c +#define STM32F429_PA1_FUNC_EVENTOUT 0x110 +#define STM32F429_PA1_FUNC_ANALOG 0x111 + +#define STM32F429_PA2_FUNC_GPIO 0x200 +#define STM32F429_PA2_FUNC_TIM2_CH3 0x202 +#define STM32F429_PA2_FUNC_TIM5_CH3 0x203 +#define STM32F429_PA2_FUNC_TIM9_CH1 0x204 +#define STM32F429_PA2_FUNC_USART2_TX 0x208 +#define STM32F429_PA2_FUNC_ETH_MDIO 0x20c +#define STM32F429_PA2_FUNC_EVENTOUT 0x210 +#define STM32F429_PA2_FUNC_ANALOG 0x211 + +#define STM32F429_PA3_FUNC_GPIO 0x300 +#define STM32F429_PA3_FUNC_TIM2_CH4 0x302 +#define STM32F429_PA3_FUNC_TIM5_CH4 0x303 +#define STM32F429_PA3_FUNC_TIM9_CH2 0x304 +#define STM32F429_PA3_FUNC_USART2_RX 0x308 +#define STM32F429_PA3_FUNC_OTG_HS_ULPI_D0 0x30b +#define STM32F429_PA3_FUNC_ETH_MII_COL 0x30c +#define STM32F429_PA3_FUNC_LCD_B5 0x30f +#define STM32F429_PA3_FUNC_EVENTOUT 0x310 +#define STM32F429_PA3_FUNC_ANALOG 0x311 + +#define STM32F429_PA4_FUNC_GPIO 0x400 +#define STM32F429_PA4_FUNC_SPI1_NSS 0x406 +#define STM32F429_PA4_FUNC_SPI3_NSS_I2S3_WS 0x407 +#define STM32F429_PA4_FUNC_USART2_CK 0x408 +#define STM32F429_PA4_FUNC_OTG_HS_SOF 0x40d +#define STM32F429_PA4_FUNC_DCMI_HSYNC 0x40e +#define STM32F429_PA4_FUNC_LCD_VSYNC 0x40f +#define STM32F429_PA4_FUNC_EVENTOUT 0x410 +#define STM32F429_PA4_FUNC_ANALOG 0x411 + +#define STM32F429_PA5_FUNC_GPIO 0x500 +#define STM32F429_PA5_FUNC_TIM2_CH1_TIM2_ETR 0x502 +#define STM32F429_PA5_FUNC_TIM8_CH1N 0x504 +#define STM32F429_PA5_FUNC_SPI1_SCK 0x506 +#define STM32F429_PA5_FUNC_OTG_HS_ULPI_CK 0x50b +#define STM32F429_PA5_FUNC_EVENTOUT 0x510 +#define STM32F429_PA5_FUNC_ANALOG 0x511 + +#define STM32F429_PA6_FUNC_GPIO 0x600 +#define STM32F429_PA6_FUNC_TIM1_BKIN 0x602 +#define STM32F429_PA6_FUNC_TIM3_CH1 0x603 +#define STM32F429_PA6_FUNC_TIM8_BKIN 0x604 +#define STM32F429_PA6_FUNC_SPI1_MISO 0x606 +#define STM32F429_PA6_FUNC_TIM13_CH1 0x60a +#define STM32F429_PA6_FUNC_DCMI_PIXCLK 0x60e +#define STM32F429_PA6_FUNC_LCD_G2 0x60f +#define STM32F429_PA6_FUNC_EVENTOUT 0x610 +#define STM32F429_PA6_FUNC_ANALOG 0x611 + +#define STM32F429_PA7_FUNC_GPIO 0x700 +#define STM32F429_PA7_FUNC_TIM1_CH1N 0x702 +#define STM32F429_PA7_FUNC_TIM3_CH2 0x703 +#define STM32F429_PA7_FUNC_TIM8_CH1N 0x704 +#define STM32F429_PA7_FUNC_SPI1_MOSI 0x706 +#define STM32F429_PA7_FUNC_TIM14_CH1 0x70a +#define STM32F429_PA7_FUNC_ETH_MII_RX_DV_ETH_RMII_CRS_DV 0x70c +#define STM32F429_PA7_FUNC_EVENTOUT 0x710 +#define STM32F429_PA7_FUNC_ANALOG 0x711 + +#define STM32F429_PA8_FUNC_GPIO 0x800 +#define STM32F429_PA8_FUNC_MCO1 0x801 +#define STM32F429_PA8_FUNC_TIM1_CH1 0x802 +#define STM32F429_PA8_FUNC_I2C3_SCL 0x805 +#define STM32F429_PA8_FUNC_USART1_CK 0x808 +#define STM32F429_PA8_FUNC_OTG_FS_SOF 0x80b +#define STM32F429_PA8_FUNC_LCD_R6 0x80f +#define STM32F429_PA8_FUNC_EVENTOUT 0x810 +#define STM32F429_PA8_FUNC_ANALOG 0x811 + +#define STM32F429_PA9_FUNC_GPIO 0x900 +#define STM32F429_PA9_FUNC_TIM1_CH2 0x902 +#define STM32F429_PA9_FUNC_I2C3_SMBA 0x905 +#define STM32F429_PA9_FUNC_USART1_TX 0x908 +#define STM32F429_PA9_FUNC_DCMI_D0 0x90e +#define STM32F429_PA9_FUNC_EVENTOUT 0x910 +#define STM32F429_PA9_FUNC_ANALOG 0x911 + +#define STM32F429_PA10_FUNC_GPIO 0xa00 +#define STM32F429_PA10_FUNC_TIM1_CH3 0xa02 +#define STM32F429_PA10_FUNC_USART1_RX 0xa08 +#define STM32F429_PA10_FUNC_OTG_FS_ID 0xa0b +#define STM32F429_PA10_FUNC_DCMI_D1 0xa0e +#define STM32F429_PA10_FUNC_EVENTOUT 0xa10 +#define STM32F429_PA10_FUNC_ANALOG 0xa11 + +#define STM32F429_PA11_FUNC_GPIO 0xb00 +#define STM32F429_PA11_FUNC_TIM1_CH4 0xb02 +#define STM32F429_PA11_FUNC_USART1_CTS 0xb08 +#define STM32F429_PA11_FUNC_CAN1_RX 0xb0a +#define STM32F429_PA11_FUNC_OTG_FS_DM 0xb0b +#define STM32F429_PA11_FUNC_LCD_R4 0xb0f +#define STM32F429_PA11_FUNC_EVENTOUT 0xb10 +#define STM32F429_PA11_FUNC_ANALOG
[PATCH v5 1/9] ARM: Kconfig: Introduce MACH_STM32F429 flag
This patch introduces the MACH_STM32F429 to make possible to only select STM32F429 pinctrl driver. By default, all the MACH_STM32Fxxx flags will be set with STM32 defconfig. Acked-by: Patrice Chotard Acked-by: Linus Walleij Signed-off-by: Maxime Coquelin --- arch/arm/Kconfig | 5 + 1 file changed, 5 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 4f799e5..ca16120 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -883,6 +883,11 @@ config ARCH_STM32 help Support for STMicroelectronics STM32 processors. +config MACH_STM32F429 + bool "STMicrolectronics STM32F429" + depends on ARCH_STM32 + default y + # Definitions to make life easier config ARCH_ACORN bool -- 1.9.1
[PATCH v5 4/9] pinctrl: Add STM32 MCUs support
This patch adds pinctrl and GPIO support to STMicroelectronic's STM32
family of MCUs.
While it only supports STM32F429 for now, it has been designed to enable
support of other MCUs of the family (e.g. STM32F746).
Acked-by: Patrice Chotard
Signed-off-by: Maxime Coquelin
---
drivers/pinctrl/Kconfig |1 +
drivers/pinctrl/Makefile |1 +
drivers/pinctrl/stm32/Kconfig | 16 +
drivers/pinctrl/stm32/Makefile|5 +
drivers/pinctrl/stm32/pinctrl-stm32.c | 832 +++
drivers/pinctrl/stm32/pinctrl-stm32.h | 51 +
drivers/pinctrl/stm32/pinctrl-stm32f429.c | 1598 +
7 files changed, 2504 insertions(+)
create mode 100644 drivers/pinctrl/stm32/Kconfig
create mode 100644 drivers/pinctrl/stm32/Makefile
create mode 100644 drivers/pinctrl/stm32/pinctrl-stm32.c
create mode 100644 drivers/pinctrl/stm32/pinctrl-stm32.h
create mode 100644 drivers/pinctrl/stm32/pinctrl-stm32f429.c
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 99a4c10..f8405cd 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -257,6 +257,7 @@ source "drivers/pinctrl/qcom/Kconfig"
source "drivers/pinctrl/samsung/Kconfig"
source "drivers/pinctrl/sh-pfc/Kconfig"
source "drivers/pinctrl/spear/Kconfig"
+source "drivers/pinctrl/stm32/Kconfig"
source "drivers/pinctrl/sunxi/Kconfig"
source "drivers/pinctrl/uniphier/Kconfig"
source "drivers/pinctrl/vt8500/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index bf1b5ca..7c3fd63 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_ARCH_QCOM) += qcom/
obj-$(CONFIG_PINCTRL_SAMSUNG) += samsung/
obj-$(CONFIG_PINCTRL_SH_PFC) += sh-pfc/
obj-$(CONFIG_PINCTRL_SPEAR)+= spear/
+obj-$(CONFIG_PINCTRL_STM32)+= stm32/
obj-$(CONFIG_ARCH_SUNXI) += sunxi/
obj-$(CONFIG_PINCTRL_UNIPHIER) += uniphier/
obj-$(CONFIG_ARCH_VT8500) += vt8500/
diff --git a/drivers/pinctrl/stm32/Kconfig b/drivers/pinctrl/stm32/Kconfig
new file mode 100644
index 000..0f28841
--- /dev/null
+++ b/drivers/pinctrl/stm32/Kconfig
@@ -0,0 +1,16 @@
+if ARCH_STM32 || COMPILE_TEST
+
+config PINCTRL_STM32
+ bool
+ depends on OF
+ select PINMUX
+ select GENERIC_PINCONF
+ select GPIOLIB
+
+config PINCTRL_STM32F429
+ bool "STMicroelectronics STM32F429 pin control" if COMPILE_TEST &&
!MACH_STM32F429
+ depends on OF
+ default MACH_STM32F429
+ select PINCTRL_STM32
+
+endif
diff --git a/drivers/pinctrl/stm32/Makefile b/drivers/pinctrl/stm32/Makefile
new file mode 100644
index 000..fc17d42
--- /dev/null
+++ b/drivers/pinctrl/stm32/Makefile
@@ -0,0 +1,5 @@
+# Core
+obj-$(CONFIG_PINCTRL_STM32) += pinctrl-stm32.o
+
+# SoC Drivers
+obj-$(CONFIG_PINCTRL_STM32F429)+= pinctrl-stm32f429.o
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c
b/drivers/pinctrl/stm32/pinctrl-stm32.c
new file mode 100644
index 000..9e8bc8f
--- /dev/null
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -0,0 +1,832 @@
+/*
+ * Copyright (C) Maxime Coquelin 2015
+ * Author: Maxime Coquelin
+ * License terms: GNU General Public License (GPL), version 2
+ *
+ * Heavily based on Mediatek's pinctrl driver
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "../core.h"
+#include "../pinconf.h"
+#include "../pinctrl-utils.h"
+#include "pinctrl-stm32.h"
+
+#define STM32_GPIO_MODER 0x00
+#define STM32_GPIO_TYPER 0x04
+#define STM32_GPIO_SPEEDR 0x08
+#define STM32_GPIO_PUPDR 0x0c
+#define STM32_GPIO_IDR 0x10
+#define STM32_GPIO_ODR 0x14
+#define STM32_GPIO_BSRR0x18
+#define STM32_GPIO_LCKR0x1c
+#define STM32_GPIO_AFRL0x20
+#define STM32_GPIO_AFRH0x24
+
+#define STM32_GPIO_PINS_PER_BANK 16
+
+#define gpio_range_to_bank(chip) \
+ container_of(chip, struct stm32_gpio_bank, range)
+
+#define gpio_chip_to_bank(chip) \
+ container_of(chip, struct stm32_gpio_bank, gpio_chip)
+
+static const char * const stm32_gpio_functions[] = {
+ "gpio", "af0", "af1",
+ "af2", "af3", "af4",
+ "af5", "af6", "af7",
+ "af8", "af9", "af10",
+ "af11", "af12", "af13",
+ "af14", "af15", "analog",
+};
+
+struct stm32_pinctrl_group {
+ const char *name;
+ unsigned long config;
+ unsigned pin;
+};
+
+struct stm32_gpio_bank {
+
[PATCH v5 8/9] ARM: dts: Add leds support to STM32F429 boards
Acked-by: Patrice Chotard
Acked-by: Linus Walleij
Signed-off-by: Maxime Coquelin
---
arch/arm/boot/dts/stm32429i-eval.dts | 17 +
arch/arm/boot/dts/stm32f429-disco.dts | 11 +++
2 files changed, 28 insertions(+)
diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
b/arch/arm/boot/dts/stm32429i-eval.dts
index 71fe17a..e3921d9 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -64,6 +64,23 @@
aliases {
serial0 = &usart1;
};
+
+ leds {
+ compatible = "gpio-leds";
+ green {
+ gpios = <&gpiog 6 1>;
+ linux,default-trigger = "heartbeat";
+ };
+ orange {
+ gpios = <&gpiog 7 1>;
+ };
+ red {
+ gpios = <&gpiog 10 1>;
+ };
+ blue {
+ gpios = <&gpiog 12 1>;
+ };
+ };
};
&clk_hse {
diff --git a/arch/arm/boot/dts/stm32f429-disco.dts
b/arch/arm/boot/dts/stm32f429-disco.dts
index e3ce796..0140807 100644
--- a/arch/arm/boot/dts/stm32f429-disco.dts
+++ b/arch/arm/boot/dts/stm32f429-disco.dts
@@ -64,6 +64,17 @@
aliases {
serial0 = &usart1;
};
+
+ leds {
+ compatible = "gpio-leds";
+ red {
+ gpios = <&gpiog 14 0>;
+ };
+ green {
+ gpios = <&gpiog 13 0>;
+ linux,default-trigger = "heartbeat";
+ };
+ };
};
&clk_hse {
--
1.9.1
[PATCH v5 9/9] ARM: config: Enable GPIO Led driver in stm32_defconfig
Acked-by: Linus Walleij Acked-by: Patrice Chotard Signed-off-by: Maxime Coquelin --- arch/arm/configs/stm32_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index ec52505..1e5ec2a 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -52,6 +52,7 @@ CONFIG_SERIAL_STM32_CONSOLE=y # CONFIG_USB_SUPPORT is not set CONFIG_NEW_LEDS=y CONFIG_LEDS_CLASS=y +CONFIG_LEDS_GPIO=y CONFIG_LEDS_TRIGGERS=y CONFIG_LEDS_TRIGGER_HEARTBEAT=y CONFIG_DMADEVICES=y -- 1.9.1
[PATCH v5 7/9] ARM: dts: Add USART1 pin config to STM32F429 boards
This patch selects USART1 pin configuration on PA9/PA10 pins
for both Eval and Disco boards.
Acked-by: Linus Walleij
Acked-by: Patrice Chotard
Signed-off-by: Maxime Coquelin
---
arch/arm/boot/dts/stm32429i-eval.dts | 2 ++
arch/arm/boot/dts/stm32f429-disco.dts | 2 ++
arch/arm/boot/dts/stm32f429.dtsi | 13 +
3 files changed, 17 insertions(+)
diff --git a/arch/arm/boot/dts/stm32429i-eval.dts
b/arch/arm/boot/dts/stm32429i-eval.dts
index 6964fc9..71fe17a 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -71,5 +71,7 @@
};
&usart1 {
+ pinctrl-0 = <&usart1_pins_a>;
+ pinctrl-names = "default";
status = "okay";
};
diff --git a/arch/arm/boot/dts/stm32f429-disco.dts
b/arch/arm/boot/dts/stm32f429-disco.dts
index f0b731d..e3ce796 100644
--- a/arch/arm/boot/dts/stm32f429-disco.dts
+++ b/arch/arm/boot/dts/stm32f429-disco.dts
@@ -71,5 +71,7 @@
};
&usart1 {
+ pinctrl-0 = <&usart1_pins_a>;
+ pinctrl-names = "default";
status = "okay";
};
diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 62d2b3d..1cf713e 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -263,6 +263,19 @@
clocks = <&rcc 0 266>;
st,bank-name = "GPIOK";
};
+
+ usart1_pins_a: usart1@0 {
+ pins1 {
+ pinmux = ;
+ bias-disable;
+ drive-push-pull;
+ slew-rate = <0>;
+ };
+ pins2 {
+ pinmux =
;
+ bias-disable;
+ };
+ };
};
rcc: rcc@40023810 {
--
1.9.1
