[PATCH 3/3] nvme: remove single trailing whitespace
There is a single trailing whitespace in core.c. Since this is just a single whitespace, the chances of this affecting backports to stable should be quite low, so let's just remove it. Signed-off-by: Niklas Cassel --- drivers/nvme/host/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index c739e4c5b621..288ac47ff5b4 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -2909,7 +2909,7 @@ int nvme_init_ctrl_finish(struct nvme_ctrl *ctrl) ret = nvme_configure_apst(ctrl); if (ret < 0) return ret; - + ret = nvme_configure_timestamp(ctrl); if (ret < 0) return ret; -- 2.30.2
[PATCH 2/3] nvme-multipath: remove single trailing whitespace
There is a single trailing whitespace in multipath.c.
Since this is just a single whitespace, the chances of this affecting
backports to stable should be quite low, so let's just remove it.
Signed-off-by: Niklas Cassel
---
drivers/nvme/host/multipath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 2bbc1685799d..68918ea1d3d0 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -697,7 +697,7 @@ void nvme_mpath_add_disk(struct nvme_ns *ns, struct
nvme_id_ns *id)
queue_work(nvme_wq, >ctrl->ana_work);
}
} else {
- ns->ana_state = NVME_ANA_OPTIMIZED;
+ ns->ana_state = NVME_ANA_OPTIMIZED;
nvme_mpath_set_live(ns);
}
--
2.30.2
[PATCH 0/3] nvme trailing whitespace cleanup
Hello nvme peeps, This series removes all the trailing whitespace I could find using: git grep '[[:blank:]]$' drivers/nvme So this should remove all the existing trailing whitespace in drivers/nvme/* Kind regards, Niklas Niklas Cassel (3): nvme-pci: remove single trailing whitespace nvme-multipath: remove single trailing whitespace nvme: remove single trailing whitespace drivers/nvme/host/core.c | 2 +- drivers/nvme/host/multipath.c | 2 +- drivers/nvme/host/pci.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) -- 2.30.2
[PATCH 1/3] nvme-pci: remove single trailing whitespace
There is a single trailing whitespace in pci.c.
Since this is just a single whitespace, the chances of this affecting
backports to stable should be quite low, so let's just remove it.
Signed-off-by: Niklas Cassel
---
drivers/nvme/host/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index b06e685d1250..09d4c5f99fc3 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2172,7 +2172,7 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)
if (nr_io_queues == 0)
return 0;
-
+
clear_bit(NVMEQ_ENABLED, >flags);
if (dev->cmb_use_sqes) {
--
2.30.2
[PATCH] nvme-pci: don't simple map sgl when sgls are disabled
From: Niklas Cassel
According to the module parameter description for sgl_threshold,
a value of 0 means that SGLs are disabled.
If SGLs are disabled, we should respect that, even for the case
where the request is made up of a single physical segment.
Fixes: 297910571f08 ("nvme-pci: optimize mapping single segment requests using
SGLs")
Signed-off-by: Niklas Cassel
---
drivers/nvme/host/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index d47bb18b976a..b06e685d1250 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -854,7 +854,7 @@ static blk_status_t nvme_map_data(struct nvme_dev *dev,
struct request *req,
return nvme_setup_prp_simple(dev, req,
>rw, );
- if (iod->nvmeq->qid &&
+ if (iod->nvmeq->qid && sgl_threshold &&
dev->ctrl.sgls & ((1 << 0) | (1 << 1)))
return nvme_setup_sgl_simple(dev, req,
>rw, );
--
2.30.2
[PATCH v3] nvme: disallow passthru cmd from targeting a nsid != nsid of the block dev
From: Niklas Cassel
When a passthru command targets a specific namespace, the ns parameter to
nvme_user_cmd()/nvme_user_cmd64() is set. However, there is currently no
validation that the nsid specified in the passthru command targets the
namespace/nsid represented by the block device that the ioctl was
performed on.
Add a check that validates that the nsid in the passthru command matches
that of the supplied namespace.
Signed-off-by: Niklas Cassel
Reviewed-by: Sagi Grimberg
Reviewed-by: Kanchan Joshi
Reviewed-by: Javier González
---
Changes since v2:
-Picked up Reviewed-by-tags.
-Send from a mail server that doesn't mangle the mail.
drivers/nvme/host/core.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f13eb4ded95f..a50352ea3f7b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1599,6 +1599,12 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct
nvme_ns *ns,
return -EFAULT;
if (cmd.flags)
return -EINVAL;
+ if (ns && cmd.nsid != ns->head->ns_id) {
+ dev_err(ctrl->device,
+ "%s: nsid (%u) in cmd does not match nsid (%u) of
namespace\n",
+ current->comm, cmd.nsid, ns->head->ns_id);
+ return -EINVAL;
+ }
memset(, 0, sizeof(c));
c.common.opcode = cmd.opcode;
@@ -1643,6 +1649,12 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl,
struct nvme_ns *ns,
return -EFAULT;
if (cmd.flags)
return -EINVAL;
+ if (ns && cmd.nsid != ns->head->ns_id) {
+ dev_err(ctrl->device,
+ "%s: nsid (%u) in cmd does not match nsid (%u) of
namespace\n",
+ current->comm, cmd.nsid, ns->head->ns_id);
+ return -EINVAL;
+ }
memset(, 0, sizeof(c));
c.common.opcode = cmd.opcode;
--
2.30.2
Re: [RFC PATCH] nvme: allow NVME_IOCTL_IO_CMD on controller char dev even when multiple ns
On Tue, Mar 30, 2021 at 08:30:22PM +0200, jav...@javigon.com wrote:
> On 26.03.2021 20:59, Niklas Cassel wrote:
> > From: Niklas Cassel
> >
> > Currently when doing NVME_IOCTL_IO_CMD on the controller character device,
> > the command is rejected if there is more than one namespace in the
> > ctrl->namespaces list.
> >
> > There is not really any reason for this restriction.
> > Instead, check the nsid value specified in the passthru command, and try
> > to find the matching namespace in ctrl->namespaces list.
> > If found, call nvme_user_cmd() on the namespace.
> > If not found, reject the command.
> >
> > While at it, remove the warning that says that NVME_IOCTL_IO_CMD is
> > deprecated on the controller character device.
> > There is no comment saying why it is deprecated.
> > It might be very unsafe to send a passthru command, but if that is
> > the issue with this IOCTL, then we should add a warning about that
> > instead.
> >
> > Signed-off-by: Niklas Cassel
>
> I think the idea is OK, but I have 3 questions:
>
> 1. Wouldn't this break user-space when nsid is not specified?
Since this is an ioctl, the kernel will always read some value
from cmd.nsid, so I assume you mean when specifying cmd.nsid == 0.
I don't think we have anything to worry about because:
a)
Like Keith said in the other thread:
"There are no IO commands accepting a 0 NSID, so rejecting those from the
driver should be okay."
Currently, when sending a NVME_IOCTL_IO_CMD on the ctrl char dev with
cmd.nsid == 0, we will take the first namespace in the list, use the
request_queue of that namespace, and then send the command there.
Since there are no I/O commands that accept a NSID == 0, whatever you
specified in cmd.opcode, you should get an "Invalid Namespace or Format"
error back from the controller.
I don't think that there is any harm in adding a check (which is essentially
what this RFC does), that will reject the command before sending it down to
the controller.
(A potential improvement in the future, on top of this patch, is to allow
nsid.cmd == broadcast address, but that is out of scope for this patch.
And no, since the current behavior on master does reject any cmd when there
is more than one namespace attached to the controller (more than one ns in
ctrl->namespaces list), I wouldn't say that master handles this case.)
b)
If you use nvme-cli then all commands that calls nvme_submit_io_passthru()
already does:
if (!cfg.namespace_id) {
err = cfg.namespace_id = nvme_get_nsid(fd);
if (err < 0) {
perror("get-namespace-id");
goto close_fd;
}
}
So either if you do a:
nvme write-zeroes -s 0 -c 0 --namespace-id=0 /dev/nvme0
or if you completely omit --namespace-id:
nvme write-zeroes -s 0 -c 0 /dev/nvme0
nvme-cli will already reject it (since the controller char device does not
(and can not) implement NVME_IOCTL_ID).
Sure, nvme-cli is just a single user of this ioctl (there might be other
users), but it is probably the most common one.
If nvme-cli already rejects it in user space, and we concluded that the
controller will reject it, I think it should be safe to reject it also
at the kernel side.
Without this patch, we are already rejecting any command, to any nsid,
if the controller has more than one namespace attached, which I think
makes less sense.
> 2. What is the use case for this? As I understand it, this char device
> is primarily for admin commands sent to the controller. Do you see a
> use case for sending commands to the namespace using the controller
> char device?
I don't have any use case for this, more than allowing to specify whatever
nsid you want in the --namespace-id parameter for nvme, when opening
/dev/nvme0, even when the controller has more than one namespace.
Why allow NVME_IOCTL_IO_CMD on /dev/nvme0 when the controller has one
namespace attached, but not when it has more than one namespace attached?
Doesn't make sense to me.
> 3. Following up on the above, if the use-case is I/O, don't you think
> it is cleaner to use the ongoing per-namespace char device effort? We
> would very much like to get your input there and eventually send a
> series together. When this is merged, we could wire that logic to the
> controller char device if there is an use-case for it.
While I'm not against your per-namespace char device effort, I'm not sure
if clean is the word I would use to decribe creating a bunch of extra
character devices, that almost no one will use, in the root of /dev/ even
(which is what the current proposal suggests.)
Perhaps it would be better if you had to do a
echo $nsid > /sys/class/nvme/nvme0/export_unsupported_namespace
to actually create one
[RFC PATCH] nvme: allow NVME_IOCTL_IO_CMD on controller char dev even when multiple ns
From: Niklas Cassel
Currently when doing NVME_IOCTL_IO_CMD on the controller character device,
the command is rejected if there is more than one namespace in the
ctrl->namespaces list.
There is not really any reason for this restriction.
Instead, check the nsid value specified in the passthru command, and try
to find the matching namespace in ctrl->namespaces list.
If found, call nvme_user_cmd() on the namespace.
If not found, reject the command.
While at it, remove the warning that says that NVME_IOCTL_IO_CMD is
deprecated on the controller character device.
There is no comment saying why it is deprecated.
It might be very unsafe to send a passthru command, but if that is
the issue with this IOCTL, then we should add a warning about that
instead.
Signed-off-by: Niklas Cassel
---
drivers/nvme/host/core.c | 30 +-
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a50352ea3f7b..b50fdf143b90 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3264,35 +3264,31 @@ static int nvme_dev_release(struct inode *inode, struct
file *file)
static int nvme_dev_user_cmd(struct nvme_ctrl *ctrl, void __user *argp)
{
+ struct nvme_passthru_cmd cmd;
struct nvme_ns *ns;
int ret;
down_read(>namespaces_rwsem);
if (list_empty(>namespaces)) {
- ret = -ENOTTY;
- goto out_unlock;
+ up_read(>namespaces_rwsem);
+ return -ENOTTY;
}
+ up_read(>namespaces_rwsem);
- ns = list_first_entry(>namespaces, struct nvme_ns, list);
- if (ns != list_last_entry(>namespaces, struct nvme_ns, list)) {
- dev_warn(ctrl->device,
- "NVME_IOCTL_IO_CMD not supported when multiple
namespaces present!\n");
- ret = -EINVAL;
- goto out_unlock;
- }
+ if (copy_from_user(, argp, sizeof(cmd)))
+ return -EFAULT;
- dev_warn(ctrl->device,
- "using deprecated NVME_IOCTL_IO_CMD ioctl on the char
device!\n");
- kref_get(>kref);
- up_read(>namespaces_rwsem);
+ ns = nvme_find_get_ns(ctrl, cmd.nsid);
+ if (!ns) {
+ dev_err(ctrl->device,
+ "%s: could not find namespace with nsid %u\n",
+ current->comm, cmd.nsid);
+ return -EINVAL;
+ }
ret = nvme_user_cmd(ctrl, ns, argp);
nvme_put_ns(ns);
return ret;
-
-out_unlock:
- up_read(>namespaces_rwsem);
- return ret;
}
static long nvme_dev_ioctl(struct file *file, unsigned int cmd,
--
2.30.2
Re: [PATCH] nvme: disallow passthru cmd from targeting a nsid != nsid of the block dev
On Fri, Mar 26, 2021 at 12:19:42AM +0900, Keith Busch wrote: > On Thu, Mar 25, 2021 at 09:48:37AM +0000, Niklas Cassel wrote: > > From: Niklas Cassel > > > > When a passthru command targets a specific namespace, the ns parameter to > > nvme_user_cmd()/nvme_user_cmd64() is set. However, there is currently no > > validation that the nsid specified in the passthru command targets the > > namespace/nsid represented by the block device that the ioctl was > > performed on. > > > > Add a check that validates that the nsid in the passthru command matches > > that of the supplied namespace. > > > > Signed-off-by: Niklas Cassel > > --- > > Currently, if doing NVME_IOCTL_IO_CMD on the controller char device, > > if and only if there is a single namespace in the ctrl->namespaces list, > > nvme_dev_user_cmd() will call nvme_user_cmd() with ns parameter set. > > While it might be good that we validate the nsid even in this case, > > perhaps we want to allow a nsid value in the passthru command to be > > 0x0 and/or the NSID broadcast value? (Only when NVME_IOCTL_IO_CMD was > > done on the controller char device though.) > > There are no IO commands accepting a 0 NSID, so rejecting those from the > driver should be okay. > > FLUSH is the only IO command that takes a broadcast NSID. I suspect at > least some entities were unfortunately sending broadcast flush through > this interface, so it's possible we'll hear of breakage, but I'd agree > your patch is still the right thing to do. I don't think this should be a problem. You shouldn't be sending a broadcast NSID via the per namespace block device. It just seems silly to specify a specific namespace block device, and then use the broadcast NSID. (This obviously should never have been allowed.) If you wanted to send a broadcast NSID, you should have used the controller character device. However, nvme_dev_user_cmd() currently rejects any NVME_IOCTL_IO_CMD when there is more than one namespace in ctrl->namespaces list, so they could never have used the controller character device to send a flush to more than one namespace. So here we don't change anything. Kind regards, Niklas
[PATCH v2] nvme: disallow passthru cmd from targeting a nsid != nsid of the block dev
From: Niklas Cassel
When a passthru command targets a specific namespace, the ns parameter to
nvme_user_cmd()/nvme_user_cmd64() is set. However, there is currently no
validation that the nsid specified in the passthru command targets the
namespace/nsid represented by the block device that the ioctl was
performed on.
Add a check that validates that the nsid in the passthru command matches
that of the supplied namespace.
Signed-off-by: Niklas Cassel
---
Changes since v1:
-Added error print.
drivers/nvme/host/core.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f13eb4ded95f..a50352ea3f7b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1599,6 +1599,12 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct
nvme_ns *ns,
return -EFAULT;
if (cmd.flags)
return -EINVAL;
+ if (ns && cmd.nsid != ns->head->ns_id) {
+ dev_err(ctrl->device,
+ "%s: nsid (%u) in cmd does not match nsid (%u) of
namespace\n",
+ current->comm, cmd.nsid, ns->head->ns_id);
+ return -EINVAL;
+ }
memset(, 0, sizeof(c));
c.common.opcode = cmd.opcode;
@@ -1643,6 +1649,12 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl,
struct nvme_ns *ns,
return -EFAULT;
if (cmd.flags)
return -EINVAL;
+ if (ns && cmd.nsid != ns->head->ns_id) {
+ dev_err(ctrl->device,
+ "%s: nsid (%u) in cmd does not match nsid (%u) of
namespace\n",
+ current->comm, cmd.nsid, ns->head->ns_id);
+ return -EINVAL;
+ }
memset(, 0, sizeof(c));
c.common.opcode = cmd.opcode;
--
2.30.2
Re: [PATCH] nvme: disallow passthru cmd from targeting a nsid != nsid of the block dev
On Thu, Mar 25, 2021 at 09:48:37AM +, Niklas Cassel wrote: > From: Niklas Cassel > > When a passthru command targets a specific namespace, the ns parameter to > nvme_user_cmd()/nvme_user_cmd64() is set. However, there is currently no > validation that the nsid specified in the passthru command targets the > namespace/nsid represented by the block device that the ioctl was > performed on. > > Add a check that validates that the nsid in the passthru command matches > that of the supplied namespace. > > Signed-off-by: Niklas Cassel > --- > Currently, if doing NVME_IOCTL_IO_CMD on the controller char device, > if and only if there is a single namespace in the ctrl->namespaces list, > nvme_dev_user_cmd() will call nvme_user_cmd() with ns parameter set. > While it might be good that we validate the nsid even in this case, > perhaps we want to allow a nsid value in the passthru command to be > 0x0 and/or the NSID broadcast value? (Only when NVME_IOCTL_IO_CMD was > done on the controller char device though.) TL;DR: Since nvme_dev_user_cmd() only calls nvme_user_cmd() if and only if there is a single namespace in the ctrl->namespaces list, I think that this patch is good as is. Long story: If we merge Javier's patch series, then we will have all namespaces (rejected or not) in ctrl->namespaces. We could then decide to remove the weird restriction that you can only use the contoller char device to do NVME_IOCTL_IO_CMD when there is one and only one entry in the ctrl->namespaces list. i.e. the user could specify any NSID, we just iterate the list and get the struct nvme_ns that matches the cmd.nsid. We could then also allow the broadcast value being specified as an NSID, and in that case, use a disk less request_queue (like Keith suggested in an earlier thread). Being allowed to specify any NSID when using the controller char device could be ok, as you probably wouldn't allow everyone access to it. So Javier's patch series still provides value, even if we allow any NSID to be specified in the controller char device, since the per namespace char devices can have more fine grained access control. Kind regards, Niklas
[PATCH] nvme: disallow passthru cmd from targeting a nsid != nsid of the block dev
From: Niklas Cassel When a passthru command targets a specific namespace, the ns parameter to nvme_user_cmd()/nvme_user_cmd64() is set. However, there is currently no validation that the nsid specified in the passthru command targets the namespace/nsid represented by the block device that the ioctl was performed on. Add a check that validates that the nsid in the passthru command matches that of the supplied namespace. Signed-off-by: Niklas Cassel --- Currently, if doing NVME_IOCTL_IO_CMD on the controller char device, if and only if there is a single namespace in the ctrl->namespaces list, nvme_dev_user_cmd() will call nvme_user_cmd() with ns parameter set. While it might be good that we validate the nsid even in this case, perhaps we want to allow a nsid value in the passthru command to be 0x0 and/or the NSID broadcast value? (Only when NVME_IOCTL_IO_CMD was done on the controller char device though.) drivers/nvme/host/core.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 40215a0246e4..e4591a4c68a8 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -1632,6 +1632,8 @@ static int nvme_user_cmd(struct nvme_ctrl *ctrl, struct nvme_ns *ns, return -EFAULT; if (cmd.flags) return -EINVAL; + if (ns && cmd.nsid != ns->head->ns_id) + return -EINVAL; memset(, 0, sizeof(c)); c.common.opcode = cmd.opcode; @@ -1676,6 +1678,8 @@ static int nvme_user_cmd64(struct nvme_ctrl *ctrl, struct nvme_ns *ns, return -EFAULT; if (cmd.flags) return -EINVAL; + if (ns && cmd.nsid != ns->head->ns_id) + return -EINVAL; memset(, 0, sizeof(c)); c.common.opcode = cmd.opcode; -- 2.30.2
Re: [PATCH] Revert "nvme: cleanup zone information initialization"
On Mon, Mar 08, 2021 at 10:02:05PM +0530, Kanchan Joshi wrote:
> On Mon, Mar 8, 2021 at 4:21 PM Niklas Cassel wrote:
> >
> > From: Niklas Cassel
> >
> > This reverts commit 73d90386b559d6f4c3c5db5e6bb1b68aae8fd3e7.
> >
> > Commit 73d90386b559 ("nvme: cleanup zone information initialization")
> > introduced the following warning at boot:
> > WARNING: CPU: 0 PID: 7 at block/blk-settings.c:252
> > blk_queue_max_zone_append_sectors+0x7d/0x90
> >
> > The warning is the result of chunk_sectors being 0.
>
> This looks same as what Chaitanya posted the fix for -
> https://lore.kernel.org/linux-nvme/20210303224717.51633-1-chaitanya.kulka...@wdc.com/
Hello Kanchan,
Indeed, thanks for telling me.
Kind regards,
Niklas
[PATCH] Revert "nvme: cleanup zone information initialization"
From: Niklas Cassel
This reverts commit 73d90386b559d6f4c3c5db5e6bb1b68aae8fd3e7.
Commit 73d90386b559 ("nvme: cleanup zone information initialization")
introduced the following warning at boot:
WARNING: CPU: 0 PID: 7 at block/blk-settings.c:252
blk_queue_max_zone_append_sectors+0x7d/0x90
The warning is the result of chunk_sectors being 0.
Worse, this causes the sysfs attribute zone_append_max_bytes to
be incorrectly reported as 0, which will break user space applications
relying on this attribute.
Looking at the commit, it probably assumes that calling
nvme_set_chunk_sectors() will cause chunk_sectors to be set.
However, looking at nvme_set_chunk_sectors(), chunk_sectors
is only set if namespace optimal i/o boundary (noiob) is non-zero.
A noiob value of zero is perfectly valid according to the spec,
and simply means that the namespace does not report an optimal i/o
boundary. Hence, we cannot assume that chunk_sectors is set after
nvme_set_chunk_sectors() has been called.
Signed-off-by: Niklas Cassel
---
drivers/nvme/host/core.c | 11 +--
drivers/nvme/host/zns.c | 11 ---
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index e68a8c4ac5a6..a38b509eeb80 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2210,18 +2210,17 @@ static int nvme_update_ns_info(struct nvme_ns *ns,
struct nvme_id_ns *id)
ns->lba_shift = id->lbaf[lbaf].ds;
nvme_set_queue_limits(ns->ctrl, ns->queue);
- ret = nvme_configure_metadata(ns, id);
- if (ret)
- goto out_unfreeze;
- nvme_set_chunk_sectors(ns, id);
- nvme_update_disk_info(ns->disk, ns, id);
-
if (ns->head->ids.csi == NVME_CSI_ZNS) {
ret = nvme_update_zone_info(ns, lbaf);
if (ret)
goto out_unfreeze;
}
+ ret = nvme_configure_metadata(ns, id);
+ if (ret)
+ goto out_unfreeze;
+ nvme_set_chunk_sectors(ns, id);
+ nvme_update_disk_info(ns->disk, ns, id);
blk_mq_unfreeze_queue(ns->disk->queue);
if (blk_queue_is_zoned(ns->queue)) {
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index c7e3ec561ba0..1dfe9a3500e3 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -9,7 +9,13 @@
int nvme_revalidate_zones(struct nvme_ns *ns)
{
- return blk_revalidate_disk_zones(ns->disk, NULL);
+ struct request_queue *q = ns->queue;
+ int ret;
+
+ ret = blk_revalidate_disk_zones(ns->disk, NULL);
+ if (!ret)
+ blk_queue_max_zone_append_sectors(q, ns->ctrl->max_zone_append);
+ return ret;
}
static int nvme_set_max_append(struct nvme_ctrl *ctrl)
@@ -103,11 +109,10 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned
lbaf)
goto free_data;
}
- blk_queue_set_zoned(ns->disk, BLK_ZONED_HM);
+ q->limits.zoned = BLK_ZONED_HM;
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
blk_queue_max_active_zones(q, le32_to_cpu(id->mar) + 1);
- blk_queue_max_zone_append_sectors(q, ns->ctrl->max_zone_append);
free_data:
kfree(id);
return status;
--
2.29.2
Re: [PATCH 1/4] power: avs: qcom-cpr: Move the driver to the qcom specific drivers
On Tue, Oct 06, 2020 at 06:05:13PM +0200, Ulf Hansson wrote: > The avs drivers are all SoC specific drivers that doesn't share any code. > Instead they are located in a directory, mostly to keep similar > functionality together. From a maintenance point of view, it makes better > sense to collect SoC specific drivers like these, into the SoC specific > directories. > > Therefore, let's move the qcom-cpr driver to the qcom directory. > > Cc: Niklas Cassel > Cc: Bjorn Andersson > Cc: Andy Gross > Cc: linux-arm-...@vger.kernel.org > Signed-off-by: Ulf Hansson > --- Acked-by: Niklas Cassel
Re: [PATCH v3] null_blk: add support for max open/active zone limit for zoned devices
On Mon, Sep 07, 2020 at 08:18:26AM +, Niklas Cassel wrote: > On Fri, Aug 28, 2020 at 12:54:00PM +0200, Niklas Cassel wrote: > > Add support for user space to set a max open zone and a max active zone > > limit via configfs. By default, the default values are 0 == no limit. > > > > Call the block layer API functions used for exposing the configured > > limits to sysfs. > > > > Add accounting in null_blk_zoned so that these new limits are respected. > > Performing an operation that would exceed these limits results in a > > standard I/O error. > > > > A max open zone limit exists in the ZBC standard. > > While null_blk_zoned is used to test the Zoned Block Device model in > > Linux, when it comes to differences between ZBC and ZNS, null_blk_zoned > > mostly follows ZBC. > > > > Therefore, implement the manage open zone resources function from ZBC, > > but additionally add support for max active zones. > > This enables user space not only to test against a device with an open > > zone limit, but also to test against a device with an active zone limit. > > > > Signed-off-by: Niklas Cassel > > Reviewed-by: Damien Le Moal > > --- > > Changes since v2: > > -Picked up Damien's Reviewed-by tag. > > -Fixed a typo in the commit message. > > -Renamed null_manage_zone_resources() to null_has_zone_resources(). > > > > drivers/block/null_blk.h | 5 + > > drivers/block/null_blk_main.c | 16 +- > > drivers/block/null_blk_zoned.c | 319 +++-- > > 3 files changed, 282 insertions(+), 58 deletions(-) > > Hello Jens, > > A gentle ping on this. > > As far as I can tell, there are no outstanding review comments. Hello Jens, Pinging you from another address, in case my corporate email is getting stuck in your spam filter. Kind regards, Niklas
Re: [PATCH v3] null_blk: add support for max open/active zone limit for zoned devices
On Fri, Aug 28, 2020 at 12:54:00PM +0200, Niklas Cassel wrote: > Add support for user space to set a max open zone and a max active zone > limit via configfs. By default, the default values are 0 == no limit. > > Call the block layer API functions used for exposing the configured > limits to sysfs. > > Add accounting in null_blk_zoned so that these new limits are respected. > Performing an operation that would exceed these limits results in a > standard I/O error. > > A max open zone limit exists in the ZBC standard. > While null_blk_zoned is used to test the Zoned Block Device model in > Linux, when it comes to differences between ZBC and ZNS, null_blk_zoned > mostly follows ZBC. > > Therefore, implement the manage open zone resources function from ZBC, > but additionally add support for max active zones. > This enables user space not only to test against a device with an open > zone limit, but also to test against a device with an active zone limit. > > Signed-off-by: Niklas Cassel > Reviewed-by: Damien Le Moal > --- > Changes since v2: > -Picked up Damien's Reviewed-by tag. > -Fixed a typo in the commit message. > -Renamed null_manage_zone_resources() to null_has_zone_resources(). > > drivers/block/null_blk.h | 5 + > drivers/block/null_blk_main.c | 16 +- > drivers/block/null_blk_zoned.c | 319 +++-- > 3 files changed, 282 insertions(+), 58 deletions(-) Hello Jens, A gentle ping on this. As far as I can tell, there are no outstanding review comments. Kind regards, Niklas
[PATCH v3] null_blk: add support for max open/active zone limit for zoned devices
Add support for user space to set a max open zone and a max active zone
limit via configfs. By default, the default values are 0 == no limit.
Call the block layer API functions used for exposing the configured
limits to sysfs.
Add accounting in null_blk_zoned so that these new limits are respected.
Performing an operation that would exceed these limits results in a
standard I/O error.
A max open zone limit exists in the ZBC standard.
While null_blk_zoned is used to test the Zoned Block Device model in
Linux, when it comes to differences between ZBC and ZNS, null_blk_zoned
mostly follows ZBC.
Therefore, implement the manage open zone resources function from ZBC,
but additionally add support for max active zones.
This enables user space not only to test against a device with an open
zone limit, but also to test against a device with an active zone limit.
Signed-off-by: Niklas Cassel
Reviewed-by: Damien Le Moal
---
Changes since v2:
-Picked up Damien's Reviewed-by tag.
-Fixed a typo in the commit message.
-Renamed null_manage_zone_resources() to null_has_zone_resources().
drivers/block/null_blk.h | 5 +
drivers/block/null_blk_main.c | 16 +-
drivers/block/null_blk_zoned.c | 319 +++--
3 files changed, 282 insertions(+), 58 deletions(-)
diff --git a/drivers/block/null_blk.h b/drivers/block/null_blk.h
index daed4a9c34367..d2e7db43a52a7 100644
--- a/drivers/block/null_blk.h
+++ b/drivers/block/null_blk.h
@@ -42,6 +42,9 @@ struct nullb_device {
struct badblocks badblocks;
unsigned int nr_zones;
+ unsigned int nr_zones_imp_open;
+ unsigned int nr_zones_exp_open;
+ unsigned int nr_zones_closed;
struct blk_zone *zones;
sector_t zone_size_sects;
@@ -51,6 +54,8 @@ struct nullb_device {
unsigned long zone_size; /* zone size in MB if device is zoned */
unsigned long zone_capacity; /* zone capacity in MB if device is zoned
*/
unsigned int zone_nr_conv; /* number of conventional zones */
+ unsigned int zone_max_open; /* max number of open zones */
+ unsigned int zone_max_active; /* max number of active zones */
unsigned int submit_queues; /* number of submission queues */
unsigned int home_node; /* home node for the device */
unsigned int queue_mode; /* block interface */
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index d74443a9c8fa2..53161a418611b 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -208,6 +208,14 @@ static unsigned int g_zone_nr_conv;
module_param_named(zone_nr_conv, g_zone_nr_conv, uint, 0444);
MODULE_PARM_DESC(zone_nr_conv, "Number of conventional zones when block device
is zoned. Default: 0");
+static unsigned int g_zone_max_open;
+module_param_named(zone_max_open, g_zone_max_open, uint, 0444);
+MODULE_PARM_DESC(zone_max_open, "Maximum number of open zones when block
device is zoned. Default: 0 (no limit)");
+
+static unsigned int g_zone_max_active;
+module_param_named(zone_max_active, g_zone_max_active, uint, 0444);
+MODULE_PARM_DESC(zone_max_active, "Maximum number of active zones when block
device is zoned. Default: 0 (no limit)");
+
static struct nullb_device *null_alloc_dev(void);
static void null_free_dev(struct nullb_device *dev);
static void null_del_dev(struct nullb *nullb);
@@ -347,6 +355,8 @@ NULLB_DEVICE_ATTR(zoned, bool, NULL);
NULLB_DEVICE_ATTR(zone_size, ulong, NULL);
NULLB_DEVICE_ATTR(zone_capacity, ulong, NULL);
NULLB_DEVICE_ATTR(zone_nr_conv, uint, NULL);
+NULLB_DEVICE_ATTR(zone_max_open, uint, NULL);
+NULLB_DEVICE_ATTR(zone_max_active, uint, NULL);
static ssize_t nullb_device_power_show(struct config_item *item, char *page)
{
@@ -464,6 +474,8 @@ static struct configfs_attribute *nullb_device_attrs[] = {
_device_attr_zone_size,
_device_attr_zone_capacity,
_device_attr_zone_nr_conv,
+ _device_attr_zone_max_open,
+ _device_attr_zone_max_active,
NULL,
};
@@ -517,7 +529,7 @@ nullb_group_drop_item(struct config_group *group, struct
config_item *item)
static ssize_t memb_group_features_show(struct config_item *item, char *page)
{
return snprintf(page, PAGE_SIZE,
-
"memory_backed,discard,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv\n");
+
"memory_backed,discard,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv,zone_max_open,zone_max_active\n");
}
CONFIGFS_ATTR_RO(memb_group_, features);
@@ -580,6 +592,8 @@ static struct nullb_device *null_alloc_dev(void)
dev->zone_size = g_zone_size;
dev->zone_capacity = g_zone_capacity;
dev->zone_nr_conv = g_zone_nr_conv;
+ dev->zone_max_open = g_zone_max_open;
+ dev->zone_max_active = g_zone_max_active;
return dev;
}
diff --git a/drivers/block/null
Re: [PATCH v2] null_blk: add support for max open/active zone limit for zoned devices
On Fri, Aug 28, 2020 at 07:06:26AM +, Damien Le Moal wrote: > On 2020/08/27 22:50, Niklas Cassel wrote: > > Add support for user space to set a max open zone and a max active zone > > limit via configfs. By default, the default values are 0 == no limit. > > > > Call the block layer API functions used for exposing the configured > > limits to sysfs. > > > > Add accounting in null_blk_zoned so that these new limits are respected. > > Performing an operating that would exceed these limits results in a > > Performing a write operation that would result in exceeding these... > > > standard I/O error. > > It is not only a write operation, also e.g. open zone operation. However I will s/Performing an operating/Performing an operation/ > > +/* > > + * This function matches the manage open zone resources function in the > > ZBC standard, > > + * with the addition of max active zones support (added in the ZNS > > standard). > > + * > > + * The function determines if a zone can transition to implicit open or > > explicit open, > > + * while maintaining the max open zone (and max active zone) limit(s). It > > may close an > > + * implicit open zone in order to make additional zone resources available. > > + * > > + * ZBC states that an implicit open zone shall be closed only if there is > > not > > + * room within the open limit. However, with the addition of an active > > limit, > > + * it is not certain that closing an implicit open zone will allow a new > > zone > > + * to be opened, since we might already be at the active limit capacity. > > + */ > > +static bool null_manage_zone_resources(struct nullb_device *dev, struct > > blk_zone *zone) > > I still do not like the name. Since this return a bool, what about > null_has_zone_resources() ? I also don't like the name :) However, since the ZBC spec, in the descriptions of "Write operation, Finish operation, and Open operation", says that the "manage open zone resources" function must be called before each of these operations are performed, and that there is a section that defines how the "manage open zone resources" is defined, I was thinking that having a similar name would be of value. And I agree that it is weird that it returns a bool, but that is how it is defined in the standard. Perhaps it should have exactly the same name as the standard, i.e. null_manage_open_zone_resources() ? However, if you don't think that there is any point of trying to have a similar name to the function in ZBC, then I will happily rename it :) Kind regards, Niklas
Re: [PATCH] null_blk: add support for max open/active zone limit for zoned devices
On Tue, Aug 25, 2020 at 11:03:58PM +, Damien Le Moal wrote: > On 2020/08/26 7:52, Damien Le Moal wrote: > > On 2020/08/25 21:22, Niklas Cassel wrote: (snip) > Arg. No, you can't. There is the trace call after the switch. So please ignore > this comment :) > > But you can still avoid repeating the "if (ret != BLK_STS_OK) return ret;" by > calling the trace only for BLK_STS_OK and returning ret at the end. Agreed. Thank you for your review comments, much appreciated. Will send out a V2 shortly. Kind regards, Niklas
[PATCH v2] null_blk: add support for max open/active zone limit for zoned devices
Add support for user space to set a max open zone and a max active zone
limit via configfs. By default, the default values are 0 == no limit.
Call the block layer API functions used for exposing the configured
limits to sysfs.
Add accounting in null_blk_zoned so that these new limits are respected.
Performing an operating that would exceed these limits results in a
standard I/O error.
A max open zone limit exists in the ZBC standard.
While null_blk_zoned is used to test the Zoned Block Device model in
Linux, when it comes to differences between ZBC and ZNS, null_blk_zoned
mostly follows ZBC.
Therefore, implement the manage open zone resources function from ZBC,
but additionally add support for max active zones.
This enables user space not only to test against a device with an open
zone limit, but also to test against a device with an active zone limit.
Signed-off-by: Niklas Cassel
---
Changes since v1:
-Fixed review comments by Damien Le Moal.
drivers/block/null_blk.h | 5 +
drivers/block/null_blk_main.c | 16 +-
drivers/block/null_blk_zoned.c | 319 +++--
3 files changed, 282 insertions(+), 58 deletions(-)
diff --git a/drivers/block/null_blk.h b/drivers/block/null_blk.h
index daed4a9c34367..d2e7db43a52a7 100644
--- a/drivers/block/null_blk.h
+++ b/drivers/block/null_blk.h
@@ -42,6 +42,9 @@ struct nullb_device {
struct badblocks badblocks;
unsigned int nr_zones;
+ unsigned int nr_zones_imp_open;
+ unsigned int nr_zones_exp_open;
+ unsigned int nr_zones_closed;
struct blk_zone *zones;
sector_t zone_size_sects;
@@ -51,6 +54,8 @@ struct nullb_device {
unsigned long zone_size; /* zone size in MB if device is zoned */
unsigned long zone_capacity; /* zone capacity in MB if device is zoned
*/
unsigned int zone_nr_conv; /* number of conventional zones */
+ unsigned int zone_max_open; /* max number of open zones */
+ unsigned int zone_max_active; /* max number of active zones */
unsigned int submit_queues; /* number of submission queues */
unsigned int home_node; /* home node for the device */
unsigned int queue_mode; /* block interface */
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index d74443a9c8fa2..53161a418611b 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -208,6 +208,14 @@ static unsigned int g_zone_nr_conv;
module_param_named(zone_nr_conv, g_zone_nr_conv, uint, 0444);
MODULE_PARM_DESC(zone_nr_conv, "Number of conventional zones when block device
is zoned. Default: 0");
+static unsigned int g_zone_max_open;
+module_param_named(zone_max_open, g_zone_max_open, uint, 0444);
+MODULE_PARM_DESC(zone_max_open, "Maximum number of open zones when block
device is zoned. Default: 0 (no limit)");
+
+static unsigned int g_zone_max_active;
+module_param_named(zone_max_active, g_zone_max_active, uint, 0444);
+MODULE_PARM_DESC(zone_max_active, "Maximum number of active zones when block
device is zoned. Default: 0 (no limit)");
+
static struct nullb_device *null_alloc_dev(void);
static void null_free_dev(struct nullb_device *dev);
static void null_del_dev(struct nullb *nullb);
@@ -347,6 +355,8 @@ NULLB_DEVICE_ATTR(zoned, bool, NULL);
NULLB_DEVICE_ATTR(zone_size, ulong, NULL);
NULLB_DEVICE_ATTR(zone_capacity, ulong, NULL);
NULLB_DEVICE_ATTR(zone_nr_conv, uint, NULL);
+NULLB_DEVICE_ATTR(zone_max_open, uint, NULL);
+NULLB_DEVICE_ATTR(zone_max_active, uint, NULL);
static ssize_t nullb_device_power_show(struct config_item *item, char *page)
{
@@ -464,6 +474,8 @@ static struct configfs_attribute *nullb_device_attrs[] = {
_device_attr_zone_size,
_device_attr_zone_capacity,
_device_attr_zone_nr_conv,
+ _device_attr_zone_max_open,
+ _device_attr_zone_max_active,
NULL,
};
@@ -517,7 +529,7 @@ nullb_group_drop_item(struct config_group *group, struct
config_item *item)
static ssize_t memb_group_features_show(struct config_item *item, char *page)
{
return snprintf(page, PAGE_SIZE,
-
"memory_backed,discard,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv\n");
+
"memory_backed,discard,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv,zone_max_open,zone_max_active\n");
}
CONFIGFS_ATTR_RO(memb_group_, features);
@@ -580,6 +592,8 @@ static struct nullb_device *null_alloc_dev(void)
dev->zone_size = g_zone_size;
dev->zone_capacity = g_zone_capacity;
dev->zone_nr_conv = g_zone_nr_conv;
+ dev->zone_max_open = g_zone_max_open;
+ dev->zone_max_active = g_zone_max_active;
return dev;
}
diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
index 3d25c9ad23831..e7e341e811fbf 100644
--- a/drivers/block/null_blk_zoned.c
+++ b/drivers/
[PATCH] null_blk: add support for max open/active zone limit for zoned devices
Add support for user space to set a max open zone and a max active zone
limit via configfs. By default, the default value is 0 == no limit.
Call the block layer API functions used for exposing the configured
limits to sysfs.
Add accounting in null_blk_zoned so that these new limits are respected.
Performing an operating that would exceed these limits results is a
standard I/O error.
A max open zone limit exists in the ZBC standard.
While null_blk_zoned is used to test the Zoned Block Device model in
Linux, when it comes to differences between ZBC and ZNS, null_blk_zoned
mostly follows ZBC.
Therefore, implement the manage open zone resources function from ZBC,
but additionally add support for max active zones.
This enables user space not only to test against a device with an open
zone limit, but also to test against a device with an active zone limit.
Signed-off-by: Niklas Cassel
---
drivers/block/null_blk.h | 5 +
drivers/block/null_blk_main.c | 16 +-
drivers/block/null_blk_zoned.c | 316 +++--
3 files changed, 283 insertions(+), 54 deletions(-)
diff --git a/drivers/block/null_blk.h b/drivers/block/null_blk.h
index daed4a9c34367..d2e7db43a52a7 100644
--- a/drivers/block/null_blk.h
+++ b/drivers/block/null_blk.h
@@ -42,6 +42,9 @@ struct nullb_device {
struct badblocks badblocks;
unsigned int nr_zones;
+ unsigned int nr_zones_imp_open;
+ unsigned int nr_zones_exp_open;
+ unsigned int nr_zones_closed;
struct blk_zone *zones;
sector_t zone_size_sects;
@@ -51,6 +54,8 @@ struct nullb_device {
unsigned long zone_size; /* zone size in MB if device is zoned */
unsigned long zone_capacity; /* zone capacity in MB if device is zoned
*/
unsigned int zone_nr_conv; /* number of conventional zones */
+ unsigned int zone_max_open; /* max number of open zones */
+ unsigned int zone_max_active; /* max number of active zones */
unsigned int submit_queues; /* number of submission queues */
unsigned int home_node; /* home node for the device */
unsigned int queue_mode; /* block interface */
diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
index d74443a9c8fa2..53161a418611b 100644
--- a/drivers/block/null_blk_main.c
+++ b/drivers/block/null_blk_main.c
@@ -208,6 +208,14 @@ static unsigned int g_zone_nr_conv;
module_param_named(zone_nr_conv, g_zone_nr_conv, uint, 0444);
MODULE_PARM_DESC(zone_nr_conv, "Number of conventional zones when block device
is zoned. Default: 0");
+static unsigned int g_zone_max_open;
+module_param_named(zone_max_open, g_zone_max_open, uint, 0444);
+MODULE_PARM_DESC(zone_max_open, "Maximum number of open zones when block
device is zoned. Default: 0 (no limit)");
+
+static unsigned int g_zone_max_active;
+module_param_named(zone_max_active, g_zone_max_active, uint, 0444);
+MODULE_PARM_DESC(zone_max_active, "Maximum number of active zones when block
device is zoned. Default: 0 (no limit)");
+
static struct nullb_device *null_alloc_dev(void);
static void null_free_dev(struct nullb_device *dev);
static void null_del_dev(struct nullb *nullb);
@@ -347,6 +355,8 @@ NULLB_DEVICE_ATTR(zoned, bool, NULL);
NULLB_DEVICE_ATTR(zone_size, ulong, NULL);
NULLB_DEVICE_ATTR(zone_capacity, ulong, NULL);
NULLB_DEVICE_ATTR(zone_nr_conv, uint, NULL);
+NULLB_DEVICE_ATTR(zone_max_open, uint, NULL);
+NULLB_DEVICE_ATTR(zone_max_active, uint, NULL);
static ssize_t nullb_device_power_show(struct config_item *item, char *page)
{
@@ -464,6 +474,8 @@ static struct configfs_attribute *nullb_device_attrs[] = {
_device_attr_zone_size,
_device_attr_zone_capacity,
_device_attr_zone_nr_conv,
+ _device_attr_zone_max_open,
+ _device_attr_zone_max_active,
NULL,
};
@@ -517,7 +529,7 @@ nullb_group_drop_item(struct config_group *group, struct
config_item *item)
static ssize_t memb_group_features_show(struct config_item *item, char *page)
{
return snprintf(page, PAGE_SIZE,
-
"memory_backed,discard,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv\n");
+
"memory_backed,discard,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv,zone_max_open,zone_max_active\n");
}
CONFIGFS_ATTR_RO(memb_group_, features);
@@ -580,6 +592,8 @@ static struct nullb_device *null_alloc_dev(void)
dev->zone_size = g_zone_size;
dev->zone_capacity = g_zone_capacity;
dev->zone_nr_conv = g_zone_nr_conv;
+ dev->zone_max_open = g_zone_max_open;
+ dev->zone_max_active = g_zone_max_active;
return dev;
}
diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
index 3d25c9ad23831..5fb38c9bdd4ae 100644
--- a/drivers/block/null_blk_zoned.c
+++ b/drivers/block/null_blk_zoned.c
@@ -51,6 +51,24 @@ int null_
[PATCH] scsi: scsi_debug: Remove superfluous close zone in resp_open_zone()
resp_open_zone() always calls zbc_open_zone() with parameter explicit set to true. If zbc_open_zone() is called with parameter explicit set to true, and the current zone state is implicit open, it will call zbc_close_zone() on the zone before proceeding. Therefore, there is no need for resp_open_zone() to call zbc_close_zone() on an implicitly open zone before calling zbc_open_zone(). Remove superfluous close zone in resp_open_zone(). Signed-off-by: Niklas Cassel --- drivers/scsi/scsi_debug.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 064ed680c0530..912d6f4878bae 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -4482,8 +4482,6 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) goto fini; } - if (zc == ZC2_IMPLICIT_OPEN) - zbc_close_zone(devip, zsp); zbc_open_zone(devip, zsp, true); fini: write_unlock(macc_lckp); -- 2.26.2
[PATCH v3 2/2] block: add max_active_zones to blk-sysfs
Add a new max_active zones definition in the sysfs documentation.
This definition will be common for all devices utilizing the zoned block
device support in the kernel.
Export max_active_zones according to this new definition for NVMe Zoned
Namespace devices, ZAC ATA devices (which are treated as SCSI devices by
the kernel), and ZBC SCSI devices.
Add the new max_active_zones member to struct request_queue, rather
than as a queue limit, since this property cannot be split across stacking
drivers.
For SCSI devices, even though max active zones is not part of the ZBC/ZAC
spec, export max_active_zones as 0, signifying "no limit".
Signed-off-by: Niklas Cassel
Reviewed-by: Javier González
Reviewed-by: Damien Le Moal
---
Documentation/ABI/testing/sysfs-block | 9 +
Documentation/block/queue-sysfs.rst | 7 +++
block/blk-sysfs.c | 14 +-
drivers/nvme/host/zns.c | 1 +
drivers/scsi/sd_zbc.c | 1 +
include/linux/blkdev.h| 25 +
6 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-block
b/Documentation/ABI/testing/sysfs-block
index f151d9cf90de..2322eb748b38 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -273,6 +273,15 @@ Description:
device ("host-aware" or "host-managed" zone model). For regular
block devices, the value is always 0.
+What: /sys/block//queue/max_active_zones
+Date: July 2020
+Contact: Niklas Cassel
+Description:
+ For zoned block devices (zoned attribute indicating
+ "host-managed" or "host-aware"), the sum of zones belonging to
+ any of the zone states: EXPLICIT OPEN, IMPLICIT OPEN or CLOSED,
+ is limited by this value. If this value is 0, there is no limit.
+
What: /sys/block//queue/max_open_zones
Date: July 2020
Contact: Niklas Cassel
diff --git a/Documentation/block/queue-sysfs.rst
b/Documentation/block/queue-sysfs.rst
index f01cf8530ae4..f261a5c84170 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -117,6 +117,13 @@ Maximum number of elements in a DMA scatter/gather list
with integrity
data that will be submitted by the block layer core to the associated
block driver.
+max_active_zones (RO)
+-
+For zoned block devices (zoned attribute indicating "host-managed" or
+"host-aware"), the sum of zones belonging to any of the zone states:
+EXPLICIT OPEN, IMPLICIT OPEN or CLOSED, is limited by this value.
+If this value is 0, there is no limit.
+
max_open_zones (RO)
---
For zoned block devices (zoned attribute indicating "host-managed" or
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 414f04579d77..7dda709f3ccb 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -311,6 +311,11 @@ static ssize_t queue_max_open_zones_show(struct
request_queue *q, char *page)
return queue_var_show(queue_max_open_zones(q), page);
}
+static ssize_t queue_max_active_zones_show(struct request_queue *q, char *page)
+{
+ return queue_var_show(queue_max_active_zones(q), page);
+}
+
static ssize_t queue_nomerges_show(struct request_queue *q, char *page)
{
return queue_var_show((blk_queue_nomerges(q) << 1) |
@@ -678,6 +683,11 @@ static struct queue_sysfs_entry queue_max_open_zones_entry
= {
.show = queue_max_open_zones_show,
};
+static struct queue_sysfs_entry queue_max_active_zones_entry = {
+ .attr = {.name = "max_active_zones", .mode = 0444 },
+ .show = queue_max_active_zones_show,
+};
+
static struct queue_sysfs_entry queue_nomerges_entry = {
.attr = {.name = "nomerges", .mode = 0644 },
.show = queue_nomerges_show,
@@ -777,6 +787,7 @@ static struct attribute *queue_attrs[] = {
_zoned_entry.attr,
_nr_zones_entry.attr,
_max_open_zones_entry.attr,
+ _max_active_zones_entry.attr,
_nomerges_entry.attr,
_rq_affinity_entry.attr,
_iostats_entry.attr,
@@ -804,7 +815,8 @@ static umode_t queue_attr_visible(struct kobject *kobj,
struct attribute *attr,
(!q->mq_ops || !q->mq_ops->timeout))
return 0;
- if (attr == _max_open_zones_entry.attr &&
+ if ((attr == _max_open_zones_entry.attr ||
+attr == _max_active_zones_entry.attr) &&
!blk_queue_is_zoned(q))
return 0;
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 3d80b9cf6bfc..57cfd78731fb 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -97,6 +97,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct
nvme_ns *ns,
q->limits.zoned
[PATCH v3 1/2] block: add max_open_zones to blk-sysfs
Add a new max_open_zones definition in the sysfs documentation.
This definition will be common for all devices utilizing the zoned block
device support in the kernel.
Export max open zones according to this new definition for NVMe Zoned
Namespace devices, ZAC ATA devices (which are treated as SCSI devices by
the kernel), and ZBC SCSI devices.
Add the new max_open_zones member to struct request_queue, rather
than as a queue limit, since this property cannot be split across stacking
drivers.
Signed-off-by: Niklas Cassel
Reviewed-by: Javier González
Reviewed-by: Damien Le Moal
---
Documentation/ABI/testing/sysfs-block | 9 +
Documentation/block/queue-sysfs.rst | 7 +++
block/blk-sysfs.c | 15 +++
drivers/nvme/host/zns.c | 1 +
drivers/scsi/sd_zbc.c | 4
include/linux/blkdev.h| 25 +
6 files changed, 61 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-block
b/Documentation/ABI/testing/sysfs-block
index ed8c14f161ee..f151d9cf90de 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -273,6 +273,15 @@ Description:
device ("host-aware" or "host-managed" zone model). For regular
block devices, the value is always 0.
+What: /sys/block//queue/max_open_zones
+Date: July 2020
+Contact: Niklas Cassel
+Description:
+ For zoned block devices (zoned attribute indicating
+ "host-managed" or "host-aware"), the sum of zones belonging to
+ any of the zone states: EXPLICIT OPEN or IMPLICIT OPEN,
+ is limited by this value. If this value is 0, there is no limit.
+
What: /sys/block//queue/chunk_sectors
Date: September 2016
Contact: Hannes Reinecke
diff --git a/Documentation/block/queue-sysfs.rst
b/Documentation/block/queue-sysfs.rst
index 6a8513af9201..f01cf8530ae4 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -117,6 +117,13 @@ Maximum number of elements in a DMA scatter/gather list
with integrity
data that will be submitted by the block layer core to the associated
block driver.
+max_open_zones (RO)
+---
+For zoned block devices (zoned attribute indicating "host-managed" or
+"host-aware"), the sum of zones belonging to any of the zone states:
+EXPLICIT OPEN or IMPLICIT OPEN, is limited by this value.
+If this value is 0, there is no limit.
+
max_sectors_kb (RW)
---
This is the maximum number of kilobytes that the block layer will allow
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index be67952e7be2..414f04579d77 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -306,6 +306,11 @@ static ssize_t queue_nr_zones_show(struct request_queue
*q, char *page)
return queue_var_show(blk_queue_nr_zones(q), page);
}
+static ssize_t queue_max_open_zones_show(struct request_queue *q, char *page)
+{
+ return queue_var_show(queue_max_open_zones(q), page);
+}
+
static ssize_t queue_nomerges_show(struct request_queue *q, char *page)
{
return queue_var_show((blk_queue_nomerges(q) << 1) |
@@ -668,6 +673,11 @@ static struct queue_sysfs_entry queue_nr_zones_entry = {
.show = queue_nr_zones_show,
};
+static struct queue_sysfs_entry queue_max_open_zones_entry = {
+ .attr = {.name = "max_open_zones", .mode = 0444 },
+ .show = queue_max_open_zones_show,
+};
+
static struct queue_sysfs_entry queue_nomerges_entry = {
.attr = {.name = "nomerges", .mode = 0644 },
.show = queue_nomerges_show,
@@ -766,6 +776,7 @@ static struct attribute *queue_attrs[] = {
_nonrot_entry.attr,
_zoned_entry.attr,
_nr_zones_entry.attr,
+ _max_open_zones_entry.attr,
_nomerges_entry.attr,
_rq_affinity_entry.attr,
_iostats_entry.attr,
@@ -793,6 +804,10 @@ static umode_t queue_attr_visible(struct kobject *kobj,
struct attribute *attr,
(!q->mq_ops || !q->mq_ops->timeout))
return 0;
+ if (attr == _max_open_zones_entry.attr &&
+ !blk_queue_is_zoned(q))
+ return 0;
+
return attr->mode;
}
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 04e5b991c00c..3d80b9cf6bfc 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -96,6 +96,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct
nvme_ns *ns,
q->limits.zoned = BLK_ZONED_HM;
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
+ blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
free_data:
kfree(id);
return status;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 183a20720da9..aa3564139b40 100644
--- a/dri
[PATCH v3 0/2] Export max open zones and max active zones to sysfs
Export max open zones and max active zones to sysfs. This patch series in based on Jens's linux-block/for-next branch. All zoned block devices in the kernel utilize the "zoned block device support" (CONFIG_BLK_DEV_ZONED). The Zoned Namespace Command Set Specification defines two different resource limits: Max Open Resources and Max Active Resources. The ZAC and ZBC standards define a MAXIMUM NUMBER OF OPEN SEQUENTIAL WRITE REQUIRED ZONES field. Since the ZNS Max Open Resources field has the same purpose as the ZAC/ZBC field, (the ZNS field is 0's based, the ZAC/ZBC field isn't), create a common "max_open_zones" definition in the sysfs documentation, and export both the ZNS field and the ZAC/ZBC field according to this new common definition. The ZNS Max Active Resources field does not have an equivalent field in ZAC/ZBC, however, since both ZAC/ZBC and ZNS utilize the "zoned block device support" in the kernel, create a "max_active_zones" definition in the sysfs documentation, similar to "max_open_zones", and export it according to this new definition. For ZAC/ZBC devices, this field will be exported as 0, meaning "no limit". Changes since v2: -Picked up Damien's Reviewed-by tags. -Update Documentation/ABI/testing/sysfs-block in addition to Documentation/block/queue-sysfs.rst (Greg). -Added bdev_max_open_zones()/bdev_max_active_zones() helpers (Johannes). Niklas Cassel (2): block: add max_open_zones to blk-sysfs block: add max_active_zones to blk-sysfs Documentation/ABI/testing/sysfs-block | 18 ++ Documentation/block/queue-sysfs.rst | 14 block/blk-sysfs.c | 27 +++ drivers/nvme/host/zns.c | 2 ++ drivers/scsi/sd_zbc.c | 5 +++ include/linux/blkdev.h| 50 +++ 6 files changed, 116 insertions(+) -- 2.26.2
Re: [PATCH v2 1/2] block: add max_open_zones to blk-sysfs
On Fri, Jul 03, 2020 at 08:22:45AM +, Johannes Thumshirn wrote:
> On 02/07/2020 20:20, Niklas Cassel wrote:
> > Documentation/block/queue-sysfs.rst | 7 +++
> > block/blk-sysfs.c | 15 +++
> > drivers/nvme/host/zns.c | 1 +
> > drivers/scsi/sd_zbc.c | 4
> > include/linux/blkdev.h | 16
> > 5 files changed, 43 insertions(+)
>
> Sorry I haven't noticed before, but you forgot null_blk.
Hello Johannes,
Actually, I haven't forgotten about null_blk :)
The problem with null_blk is that, compared to these simple patches that
simply exposes the Max Open Zones/Max Active Zones, null_blk additionally
has to keep track of all the zone accounting, and give an error if any
of these limits are exceeded.
null_blk right now follows neither the ZBC nor the ZNS specification
(even though it is almost compliant with ZBC). However, since null_blk
is really great to test with, we want it to support Max Active Zones,
even if that concept does not exist in the ZBC standard.
To add to the problem, open() does not work exactly the same in ZBC and
ZNS. In ZBC, the device always tries to close an implicit zone to make
room for an explicit zone. In ZNS, a controller that doesn't do this is
fully compliant with the spec.
So now for null_blk, you have things like zones being implicit closed when
a new zone is opened. And now we will have an additional limit (Max Active
Zones), that we need to consider before we can even try to close a zone.
I've spent a couple of days trying to implement this already, and I think
that I have a way forward. However, considering that vacations are coming
up, and that I have a bunch of other stuff that I need to do before then,
I'm not 100% sure that I will be able to finish it in time for the coming
merge window.
Therefore, I was hoping that we could merge this series as is, and I will
send out the null_blk changes when they are ready, which might or might
not make it for this merge window.
In the meantime, MAR/MOR properties for null_blk will be exposed as 0,
which means "no limit". (Which is the case when a zoned block device driver
doesn't do an explicit call to blk_queue_max_{open,active}_zones()).
Kind regards,
Niklas
[PATCH v2 2/2] block: add max_active_zones to blk-sysfs
Add a new max_active zones definition in the sysfs documentation.
This definition will be common for all devices utilizing the zoned block
device support in the kernel.
Export max_active_zones according to this new definition for NVMe Zoned
Namespace devices, ZAC ATA devices (which are treated as SCSI devices by
the kernel), and ZBC SCSI devices.
Add the new max_active_zones member to struct request_queue, rather
than as a queue limit, since this property cannot be split across stacking
drivers.
For SCSI devices, even though max active zones is not part of the ZBC/ZAC
spec, export max_active_zones as 0, signifying "no limit".
Signed-off-by: Niklas Cassel
Reviewed-by: Javier González
---
Documentation/block/queue-sysfs.rst | 7 +++
block/blk-sysfs.c | 14 +-
drivers/nvme/host/zns.c | 1 +
drivers/scsi/sd_zbc.c | 1 +
include/linux/blkdev.h | 16
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/Documentation/block/queue-sysfs.rst
b/Documentation/block/queue-sysfs.rst
index f01cf8530ae4..f261a5c84170 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -117,6 +117,13 @@ Maximum number of elements in a DMA scatter/gather list
with integrity
data that will be submitted by the block layer core to the associated
block driver.
+max_active_zones (RO)
+-
+For zoned block devices (zoned attribute indicating "host-managed" or
+"host-aware"), the sum of zones belonging to any of the zone states:
+EXPLICIT OPEN, IMPLICIT OPEN or CLOSED, is limited by this value.
+If this value is 0, there is no limit.
+
max_open_zones (RO)
---
For zoned block devices (zoned attribute indicating "host-managed" or
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index fa42961e9678..624bb4d85fc7 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -310,6 +310,11 @@ static ssize_t queue_max_open_zones_show(struct
request_queue *q, char *page)
return queue_var_show(queue_max_open_zones(q), page);
}
+static ssize_t queue_max_active_zones_show(struct request_queue *q, char *page)
+{
+ return queue_var_show(queue_max_active_zones(q), page);
+}
+
static ssize_t queue_nomerges_show(struct request_queue *q, char *page)
{
return queue_var_show((blk_queue_nomerges(q) << 1) |
@@ -677,6 +682,11 @@ static struct queue_sysfs_entry queue_max_open_zones_entry
= {
.show = queue_max_open_zones_show,
};
+static struct queue_sysfs_entry queue_max_active_zones_entry = {
+ .attr = {.name = "max_active_zones", .mode = 0444 },
+ .show = queue_max_active_zones_show,
+};
+
static struct queue_sysfs_entry queue_nomerges_entry = {
.attr = {.name = "nomerges", .mode = 0644 },
.show = queue_nomerges_show,
@@ -776,6 +786,7 @@ static struct attribute *queue_attrs[] = {
_zoned_entry.attr,
_nr_zones_entry.attr,
_max_open_zones_entry.attr,
+ _max_active_zones_entry.attr,
_nomerges_entry.attr,
_rq_affinity_entry.attr,
_iostats_entry.attr,
@@ -803,7 +814,8 @@ static umode_t queue_attr_visible(struct kobject *kobj,
struct attribute *attr,
(!q->mq_ops || !q->mq_ops->timeout))
return 0;
- if (attr == _max_open_zones_entry.attr &&
+ if ((attr == _max_open_zones_entry.attr ||
+attr == _max_active_zones_entry.attr) &&
!blk_queue_is_zoned(q))
return 0;
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 3d80b9cf6bfc..57cfd78731fb 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -97,6 +97,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct
nvme_ns *ns,
q->limits.zoned = BLK_ZONED_HM;
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
+ blk_queue_max_active_zones(q, le32_to_cpu(id->mar) + 1);
free_data:
kfree(id);
return status;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index aa3564139b40..d8b2c49d645b 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -721,6 +721,7 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char
*buf)
blk_queue_max_open_zones(q, 0);
else
blk_queue_max_open_zones(q, sdkp->zones_max_open);
+ blk_queue_max_active_zones(q, 0);
nr_zones = round_up(sdkp->capacity, zone_blocks) >> ilog2(zone_blocks);
/* READ16/WRITE16 is mandatory for ZBC disks */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index fe168abcfdda..bb9e6eb6a7e6 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -521,6 +521,7 @@ struct request_queue {
unsigned long *con
[PATCH v2 1/2] block: add max_open_zones to blk-sysfs
Add a new max_open_zones definition in the sysfs documentation.
This definition will be common for all devices utilizing the zoned block
device support in the kernel.
Export max open zones according to this new definition for NVMe Zoned
Namespace devices, ZAC ATA devices (which are treated as SCSI devices by
the kernel), and ZBC SCSI devices.
Add the new max_open_zones member to struct request_queue, rather
than as a queue limit, since this property cannot be split across stacking
drivers.
Signed-off-by: Niklas Cassel
Reviewed-by: Javier González
---
Documentation/block/queue-sysfs.rst | 7 +++
block/blk-sysfs.c | 15 +++
drivers/nvme/host/zns.c | 1 +
drivers/scsi/sd_zbc.c | 4
include/linux/blkdev.h | 16
5 files changed, 43 insertions(+)
diff --git a/Documentation/block/queue-sysfs.rst
b/Documentation/block/queue-sysfs.rst
index 6a8513af9201..f01cf8530ae4 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -117,6 +117,13 @@ Maximum number of elements in a DMA scatter/gather list
with integrity
data that will be submitted by the block layer core to the associated
block driver.
+max_open_zones (RO)
+---
+For zoned block devices (zoned attribute indicating "host-managed" or
+"host-aware"), the sum of zones belonging to any of the zone states:
+EXPLICIT OPEN or IMPLICIT OPEN, is limited by this value.
+If this value is 0, there is no limit.
+
max_sectors_kb (RW)
---
This is the maximum number of kilobytes that the block layer will allow
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 02643e149d5e..fa42961e9678 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -305,6 +305,11 @@ static ssize_t queue_nr_zones_show(struct request_queue
*q, char *page)
return queue_var_show(blk_queue_nr_zones(q), page);
}
+static ssize_t queue_max_open_zones_show(struct request_queue *q, char *page)
+{
+ return queue_var_show(queue_max_open_zones(q), page);
+}
+
static ssize_t queue_nomerges_show(struct request_queue *q, char *page)
{
return queue_var_show((blk_queue_nomerges(q) << 1) |
@@ -667,6 +672,11 @@ static struct queue_sysfs_entry queue_nr_zones_entry = {
.show = queue_nr_zones_show,
};
+static struct queue_sysfs_entry queue_max_open_zones_entry = {
+ .attr = {.name = "max_open_zones", .mode = 0444 },
+ .show = queue_max_open_zones_show,
+};
+
static struct queue_sysfs_entry queue_nomerges_entry = {
.attr = {.name = "nomerges", .mode = 0644 },
.show = queue_nomerges_show,
@@ -765,6 +775,7 @@ static struct attribute *queue_attrs[] = {
_nonrot_entry.attr,
_zoned_entry.attr,
_nr_zones_entry.attr,
+ _max_open_zones_entry.attr,
_nomerges_entry.attr,
_rq_affinity_entry.attr,
_iostats_entry.attr,
@@ -792,6 +803,10 @@ static umode_t queue_attr_visible(struct kobject *kobj,
struct attribute *attr,
(!q->mq_ops || !q->mq_ops->timeout))
return 0;
+ if (attr == _max_open_zones_entry.attr &&
+ !blk_queue_is_zoned(q))
+ return 0;
+
return attr->mode;
}
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 04e5b991c00c..3d80b9cf6bfc 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -96,6 +96,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct
nvme_ns *ns,
q->limits.zoned = BLK_ZONED_HM;
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
+ blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
free_data:
kfree(id);
return status;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 183a20720da9..aa3564139b40 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -717,6 +717,10 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned
char *buf)
/* The drive satisfies the kernel restrictions: set it up */
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
+ if (sdkp->zones_max_open == U32_MAX)
+ blk_queue_max_open_zones(q, 0);
+ else
+ blk_queue_max_open_zones(q, sdkp->zones_max_open);
nr_zones = round_up(sdkp->capacity, zone_blocks) >> ilog2(zone_blocks);
/* READ16/WRITE16 is mandatory for ZBC disks */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8fd900998b4e..fe168abcfdda 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -520,6 +520,7 @@ struct request_queue {
unsigned intnr_zones;
unsigned long *conv_zones_bitmap;
unsigned long *seq_zones_wlock;
+ unsigned intmax_open_zones;
[PATCH v2 0/2] Export max open zones and max active zones to sysfs
Export max open zones and max active zones to sysfs. Since this patch series depends on the Zoned Namespace Command Set series that has been picked up in the nvme-5.9 branch in the nvme git tree, I have based this series upon nvme-5.9. Jens, Christoph, I don't know how you usually sync stuff, perhaps the nvme-5.9 branch could be merged into linux-block/for-next, once now, and once later (like usual), to ease with integration patches like this, that changes code belonging to the block layer, but depending on commits in the nvme tree. I have a feeling that this series will not be the one series depending on the ZNS patches for this coming merge window. All zoned block devices in the kernel utilize the "zoned block device support" (CONFIG_BLK_DEV_ZONED). The Zoned Namespace Command Set Specification defines two different resource limits: Max Open Resources and Max Active Resources. The ZAC and ZBC standards define a MAXIMUM NUMBER OF OPEN SEQUENTIAL WRITE REQUIRED ZONES field. Since the ZNS Max Open Resources field has the same purpose as the ZAC/ZBC field, (the ZNS field is 0's based, the ZAC/ZBC field isn't), create a common "max_open_zones" definition in the sysfs documentation, and export both the ZNS field and the ZAC/ZBC field according to this new common definition. The ZNS Max Active Resources field does not have an equivalent field in ZAC/ZBC, however, since both ZAC/ZBC and ZNS utilize the "zoned block device support" in the kernel, create a "max_active_zones" definition in the sysfs documentation, similar to "max_open_zones", and export it according to this new definition. For ZAC/ZBC devices, this field will be exported as 0, meaning "no limit". Changes since v1: -Picked up Javier's Reviewed-by tags. -Reworded commit message (Damien). -Dropped unused stubs for setting MAR/MOR when building without CONFIG_BLK_DEV_ZONED (Damien). Niklas Cassel (2): block: add max_open_zones to blk-sysfs block: add max_active_zones to blk-sysfs Documentation/block/queue-sysfs.rst | 14 + block/blk-sysfs.c | 27 drivers/nvme/host/zns.c | 2 ++ drivers/scsi/sd_zbc.c | 5 + include/linux/blkdev.h | 32 + 5 files changed, 80 insertions(+) -- 2.26.2
Re: [PATCH 1/2] block: add max_open_zones to blk-sysfs
On Tue, Jun 30, 2020 at 01:49:41AM +, Damien Le Moal wrote:
> On 2020/06/16 19:28, Niklas Cassel wrote:
> > diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
> > index c08f6281b614..af156529f3b6 100644
> > --- a/drivers/nvme/host/zns.c
> > +++ b/drivers/nvme/host/zns.c
> > @@ -82,6 +82,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct
> > nvme_ns *ns,
> >
> > q->limits.zoned = BLK_ZONED_HM;
> > blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> > + blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
> > free_data:
> > kfree(id);
> > return status;
> > diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> > index 183a20720da9..aa3564139b40 100644
> > --- a/drivers/scsi/sd_zbc.c
> > +++ b/drivers/scsi/sd_zbc.c
> > @@ -717,6 +717,10 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned
> > char *buf)
> > /* The drive satisfies the kernel restrictions: set it up */
> > blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> > blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
> > + if (sdkp->zones_max_open == U32_MAX)
> > + blk_queue_max_open_zones(q, 0);
> > + else
> > + blk_queue_max_open_zones(q, sdkp->zones_max_open);
>
> This is correct only for host-managed drives. Host-aware models define the
> "OPTIMAL NUMBER OF OPEN SEQUENTIAL WRITE PREFERRED ZONES" instead of a maximum
> number of open sequential write required zones.
>
> Since the standard does not actually explicitly define what the value of the
> maximum number of open sequential write required zones should be for a
> host-aware drive, I would suggest to always have the max_open_zones value set
> to
> 0 for host-aware disks.
Isn't this already the case?
At least according to the comments:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/sd_zbc.c?h=v5.8-rc3#n555
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/sd_zbc.c?h=v5.8-rc3#n561
We seem to set
sdkp->zones_max_open = 0;
for host-aware, and
sdkp->zones_max_open = get_unaligned_be32([16]);
for host-managed.
So the blk_queue_max_open_zones(q, sdkp->zones_max_open) call in
sd_zbc_read_zones() should already export this new sysfs property
as 0 for host-aware disks.
Kind regards,
Niklas
>
> > nr_zones = round_up(sdkp->capacity, zone_blocks) >> ilog2(zone_blocks);
> >
> > /* READ16/WRITE16 is mandatory for ZBC disks */
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index 8fd900998b4e..2f332f00501d 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -520,6 +520,7 @@ struct request_queue {
> > unsigned intnr_zones;
> > unsigned long *conv_zones_bitmap;
> > unsigned long *seq_zones_wlock;
> > + unsigned intmax_open_zones;
> > #endif /* CONFIG_BLK_DEV_ZONED */
> >
> > /*
> > @@ -729,6 +730,17 @@ static inline bool blk_queue_zone_is_seq(struct
> > request_queue *q,
> > return true;
> > return !test_bit(blk_queue_zone_no(q, sector), q->conv_zones_bitmap);
> > }
> > +
> > +static inline void blk_queue_max_open_zones(struct request_queue *q,
> > + unsigned int max_open_zones)
> > +{
> > + q->max_open_zones = max_open_zones;
> > +}
> > +
> > +static inline unsigned int queue_max_open_zones(const struct request_queue
> > *q)
> > +{
> > + return q->max_open_zones;
> > +}
> > #else /* CONFIG_BLK_DEV_ZONED */
> > static inline unsigned int blk_queue_nr_zones(struct request_queue *q)
> > {
> > @@ -744,6 +756,14 @@ static inline unsigned int blk_queue_zone_no(struct
> > request_queue *q,
> > {
> > return 0;
> > }
> > +static inline void blk_queue_max_open_zones(struct request_queue *q,
> > + unsigned int max_open_zones)
> > +{
> > +}
>
> Why is this one necessary ? For the !CONFIG_BLK_DEV_ZONED case, no driver
> should
> ever call this function.
Will remove in v2.
Re: [PATCH 2/2] block: add max_active_zones to blk-sysfs
On Wed, Jul 01, 2020 at 01:16:52PM +0200, Javier González wrote: > On 16.06.2020 12:25, Niklas Cassel wrote: > > Add a new max_active zones definition in the sysfs documentation. > > This definition will be common for all devices utilizing the zoned block > > device support in the kernel. > > > > Export max_active_zones according to this new definition for NVMe Zoned > > Namespace devices, ZAC ATA devices (which are treated as SCSI devices by > > the kernel), and ZBC SCSI devices. > > > > Add the new max_active_zones struct member to the request_queue, rather > > than as a queue limit, since this property cannot be split across stacking > > drivers. > > > > For SCSI devices, even though max active zones is not part of the ZBC/ZAC > > spec, export max_active_zones as 0, signifying "no limit". > > > > Signed-off-by: Niklas Cassel > > --- (snip) > Looking a second time at these patches, wouldn't it make sense to move > this to queue_limits? Hello Javier, The problem with having MAR/MOR as queue_limits, is that they then would be split across stacking drivers/device-mapper targets. However, MAR/MOR are not splittable, at least not the way the block layer works today. If the block layer and drivers ever change so that they do accounting of zone conditions, then we could divide the MAR/MOR to be split over stacking drivers, but because of performance reasons, this will probably never happen. In the unlikely event that it did happen, we would still use the same sysfs-path for these properties, the only thing that would change would be that these would be moved into queue_limits. So the way the code looks right now, these properties cannot be split, therefore I chose to put them inside request_queue (just like nr_zones), rather than request_queue->limits (which is of type struct queue_limits). nr_zones is also exposed as a sysfs property, even though it is part of request_queue, so I don't see why MAR/MOR can't do the same. Also see Damien's replies to PATCH 1/2 of this series, which reaches the same conclusion. Kind regards, Niklas
Re: [PATCH 1/2] block: add zone_desc_ext_bytes to sysfs
On Mon, Jun 29, 2020 at 12:52:46AM +, Damien Le Moal wrote:
> On 2020/06/29 8:01, Matias Bjorling wrote:
> > The NVMe Zoned Namespace Command Set adds support for associating
> > data to a zone through the Zone Descriptor Extension feature.
> >
> > The Zone Descriptor Extension size is fixed to a multiple of 64
> > bytes. A value of zero communicates the feature is not available.
> > A value larger than zero communites the feature is available, and
> > the specified Zone Descriptor Extension size in bytes.
> >
> > The Zone Descriptor Extension feature is only available in the
> > NVMe Zoned Namespaces Command Set. Devices that supports ZAC/ZBC
> > therefore reports this value as zero, where as the NVMe device
> > driver reports the Zone Descriptor Extension size from the
> > specific device.
> >
> > Signed-off-by: Matias Bjørling
> > ---
> > Documentation/block/queue-sysfs.rst | 6 ++
> > block/blk-sysfs.c | 15 ++-
> > drivers/nvme/host/zns.c | 1 +
> > drivers/scsi/sd_zbc.c | 1 +
> > include/linux/blkdev.h | 22 ++
> > 5 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/block/queue-sysfs.rst
> > b/Documentation/block/queue-sysfs.rst
> > index f261a5c84170..c4fa195c87b4 100644
(snip)
> > static struct queue_sysfs_entry queue_nomerges_entry = {
> > .attr = {.name = "nomerges", .mode = 0644 },
> > .show = queue_nomerges_show,
> > @@ -787,6 +798,7 @@ static struct attribute *queue_attrs[] = {
> > _nr_zones_entry.attr,
> > _max_open_zones_entry.attr,
> > _max_active_zones_entry.attr,
>
> Which tree is this patch based on ? Not I have seen any patch introducing max
> active zones.
The cover letter forgot to mention this patch series' dependencies.
I assume that it is based on the following patch series:
https://lore.kernel.org/linux-block/20200622162530.1287650-1-kbu...@kernel.org/
https://lore.kernel.org/linux-block/20200616102546.491961-1-niklas.cas...@wdc.com/
Kind regards,
Niklas
Re: [PATCH v2 0/2] remove workarounds for gcc bug wrt unnamed fields in initializers
On Wed, Jun 24, 2020 at 10:40:21PM +, Chaitanya Kulkarni wrote: > Christoph, Sagi and Keith, > > On 6/24/20 9:44 AM, Christoph Hellwig wrote: > > This looks good to me, but I'd rather wait a few releases to > > avoid too mush backporting pain. > > > > Here is a summary, for longer explanation please have a look at the > end [1] :- > > Pros: > 1. Code looks uniform and follows strict policy. > > Cons: > 1. Adds a tab + more char [1] which can lead to line breaks and that can > be avoided without following declare-init pattern, less bugs and > no pressure to fit the initializer in ~72 char given that we do have > some long names and who knows what is in the future. [BEGIN being silly.. sorry guys, I couldn't resist.. :)] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/scripts/checkpatch.pl?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144 Isn't 100 the new 80? ;) [END being silly]
Re: [PATCH v2 0/2] remove workarounds for gcc bug wrt unnamed fields in initializers
On Wed, Jun 24, 2020 at 07:02:11PM +0200, Christoph Hellwig wrote: > On Wed, Jun 24, 2020 at 04:57:48PM +0000, Niklas Cassel wrote: > > On Wed, Jun 24, 2020 at 06:44:41PM +0200, Christoph Hellwig wrote: > > > This looks good to me, but I'd rather wait a few releases to > > > avoid too mush backporting pain. > > > > Chaitanya made me realize that about half of the nvme functions > > are using "struct nvme_command c" on the stack, and then memsets > > it, and half of the nvme functions are using an initializer. > > > > IMHO, using an initializer is more clear. > > > > memset has to be used if the function needs to reset an > > existing struct, but in none of the functions that I've seen, > > are we given an existing nvme_command that we need to reset. > > All the functions that I've seen declares a new nvme_command > > on the stack (so an initializer makes more sense). > > > > What do you think about me unifying this later on? > > I like the initializers a lot. But as I said I'd rather wait a > bit for now. Just to be clear: Even with these patches, about half of the nvme functions are using memset rather than initializers. But sure, I'll wait a couple of releases, and then rebase this, and additionally convert the "struct nvme_command c" memset users to use initializers. Kind regards, Niklas
Re: [PATCH v2 0/2] remove workarounds for gcc bug wrt unnamed fields in initializers
On Wed, Jun 24, 2020 at 06:44:41PM +0200, Christoph Hellwig wrote: > This looks good to me, but I'd rather wait a few releases to > avoid too mush backporting pain. Chaitanya made me realize that about half of the nvme functions are using "struct nvme_command c" on the stack, and then memsets it, and half of the nvme functions are using an initializer. IMHO, using an initializer is more clear. memset has to be used if the function needs to reset an existing struct, but in none of the functions that I've seen, are we given an existing nvme_command that we need to reset. All the functions that I've seen declares a new nvme_command on the stack (so an initializer makes more sense). What do you think about me unifying this later on? Kind regards, Niklas
[PATCH v2 0/2] remove workarounds for gcc bug wrt unnamed fields in initializers
Make the nvme code more uniform by initializing struct members at
declaration time. This change is done both in drivers/nvme/host/ and
drivers/nvme/target/.
This is how the design pattern was in nvme, before workarounds for a gcc
bug were introduced in commit e44ac588cd61 ("drivers/block/nvme-core.c:
fix build with gcc-4.4.4").
Since the minimum gcc version needed to build the kernel is now gcc 4.8.0,
which does not have this bug, revert to the previous design pattern,
which matches how the rest of the nvme code handles initialization
of struct members (excluding the cases where anonymous unions were
involved).
If, for some reason, we want to allow builds with gcc < 4.6.0
even though the minimum gcc version is now 4.8.0,
there is another less intrusive workaround where you add an extra pair of
curly braces, see e.g. commit 6cc65be4f6f2 ("locking/qspinlock: Fix build
for anonymous union in older GCC compilers").
Changes since v1:
-Fixed RDMA build error.
Niklas Cassel (2):
nvme: remove workarounds for gcc bug wrt unnamed fields in
initializers
nvmet: remove workarounds for gcc bug wrt unnamed fields in
initializers
drivers/nvme/host/core.c | 59 ++--
drivers/nvme/host/lightnvm.c | 32 +--
drivers/nvme/host/rdma.c | 28 -
drivers/nvme/target/rdma.c | 23 +++---
4 files changed, 71 insertions(+), 71 deletions(-)
--
2.26.2
[PATCH v2 2/2] nvmet: remove workarounds for gcc bug wrt unnamed fields in initializers
Workarounds for gcc issues with initializers and anon unions was first
introduced in commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build
with gcc-4.4.4").
The gcc bug in question has been fixed since gcc 4.6.0:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676
The minimum gcc version for building the kernel has been 4.6.0 since
commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6"),
and has since been updated to gcc 4.8.0 in
commit 5429ef62bcf3 ("compiler/gcc: Raise minimum GCC version for
kernel builds to 4.8").
For that reason, it should now be safe to remove these workarounds
and make the code look like it did before
commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build with gcc-4.4.4")
was introduced.
Signed-off-by: Niklas Cassel
---
drivers/nvme/target/rdma.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 6731e0349480..238bc55de561 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1535,19 +1535,20 @@ static int nvmet_rdma_cm_accept(struct rdma_cm_id
*cm_id,
struct nvmet_rdma_queue *queue,
struct rdma_conn_param *p)
{
- struct rdma_conn_param param = { };
- struct nvme_rdma_cm_rep priv = { };
+ struct nvme_rdma_cm_rep priv = {
+ .recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0),
+ .crqsize = cpu_to_le16(queue->recv_queue_size),
+ };
+ struct rdma_conn_param param = {
+ .rnr_retry_count = 7,
+ .flow_control = 1,
+ .initiator_depth = min_t(u8, p->initiator_depth,
+ queue->dev->device->attrs.max_qp_init_rd_atom),
+ .private_data = ,
+ .private_data_len = sizeof(priv),
+ };
int ret = -ENOMEM;
- param.rnr_retry_count = 7;
- param.flow_control = 1;
- param.initiator_depth = min_t(u8, p->initiator_depth,
- queue->dev->device->attrs.max_qp_init_rd_atom);
- param.private_data =
- param.private_data_len = sizeof(priv);
- priv.recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0);
- priv.crqsize = cpu_to_le16(queue->recv_queue_size);
-
ret = rdma_accept(cm_id, );
if (ret)
pr_err("rdma_accept failed (error code = %d)\n", ret);
--
2.26.2
[PATCH v2 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
Workarounds for gcc issues with initializers and anon unions was first
introduced in commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build
with gcc-4.4.4").
The gcc bug in question has been fixed since gcc 4.6.0:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676
The minimum gcc version for building the kernel has been 4.6.0 since
commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6"),
and has since been updated to gcc 4.8.0 in
commit 5429ef62bcf3 ("compiler/gcc: Raise minimum GCC version for
kernel builds to 4.8").
For that reason, it should now be safe to remove these workarounds
and make the code look like it did before
commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build with gcc-4.4.4")
was introduced.
Signed-off-by: Niklas Cassel
---
drivers/nvme/host/core.c | 59 ++--
drivers/nvme/host/lightnvm.c | 32 +--
drivers/nvme/host/rdma.c | 28 -
3 files changed, 59 insertions(+), 60 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9491dbcfe81a..99059340d723 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1038,13 +1038,12 @@ static bool nvme_ctrl_limited_cns(struct nvme_ctrl
*ctrl)
static int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.cns = NVME_ID_CNS_CTRL,
+ };
int error;
- /* gcc-4.4.4 (at least) has issues with initializers and anon unions */
- c.identify.opcode = nvme_admin_identify;
- c.identify.cns = NVME_ID_CNS_CTRL;
-
*id = kmalloc(sizeof(struct nvme_id_ctrl), GFP_KERNEL);
if (!*id)
return -ENOMEM;
@@ -1096,16 +1095,16 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl,
struct nvme_ns_ids *ids,
static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
struct nvme_ns_ids *ids)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.nsid = cpu_to_le32(nsid),
+ .identify.cns = NVME_ID_CNS_NS_DESC_LIST,
+ };
int status;
void *data;
int pos;
int len;
- c.identify.opcode = nvme_admin_identify;
- c.identify.nsid = cpu_to_le32(nsid);
- c.identify.cns = NVME_ID_CNS_NS_DESC_LIST;
-
data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -1143,11 +1142,12 @@ static int nvme_identify_ns_descs(struct nvme_ctrl
*ctrl, unsigned nsid,
static int nvme_identify_ns_list(struct nvme_ctrl *dev, unsigned nsid, __le32
*ns_list)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.cns = NVME_ID_CNS_NS_ACTIVE_LIST,
+ .identify.nsid = cpu_to_le32(nsid),
+ };
- c.identify.opcode = nvme_admin_identify;
- c.identify.cns = NVME_ID_CNS_NS_ACTIVE_LIST;
- c.identify.nsid = cpu_to_le32(nsid);
return nvme_submit_sync_cmd(dev->admin_q, , ns_list,
NVME_IDENTIFY_DATA_SIZE);
}
@@ -1155,14 +1155,13 @@ static int nvme_identify_ns_list(struct nvme_ctrl *dev,
unsigned nsid, __le32 *n
static int nvme_identify_ns(struct nvme_ctrl *ctrl,
unsigned nsid, struct nvme_id_ns **id)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.nsid = cpu_to_le32(nsid),
+ .identify.cns = NVME_ID_CNS_NS,
+ };
int error;
- /* gcc-4.4.4 (at least) has issues with initializers and anon unions */
- c.identify.opcode = nvme_admin_identify;
- c.identify.nsid = cpu_to_le32(nsid);
- c.identify.cns = NVME_ID_CNS_NS;
-
*id = kmalloc(sizeof(**id), GFP_KERNEL);
if (!*id)
return -ENOMEM;
@@ -2815,17 +2814,17 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl,
struct nvme_id_ctrl *id)
int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp,
void *log, size_t size, u64 offset)
{
- struct nvme_command c = { };
u32 dwlen = nvme_bytes_to_numd(size);
-
- c.get_log_page.opcode = nvme_admin_get_log_page;
- c.get_log_page.nsid = cpu_to_le32(nsid);
- c.get_log_page.lid = log_page;
- c.get_log_page.lsp = lsp;
- c.get_log_page.numdl = cpu_to_le16(dwlen & ((1 << 16) - 1));
- c.get_log_page.numdu = cpu_to_le16(dwlen >> 16);
- c.get_log_page.lpol = cpu_to_le32(lower_32_bits(offset));
- c.get_log_page.lpou = cpu_to_le32(upper_32_bits(offset));
+ str
Re: [PATCH 2/2] nvmet: remove workarounds for gcc bug wrt unnamed fields in initializers
On Thu, Jun 18, 2020 at 05:29:00PM +, Chaitanya Kulkarni wrote:
> I'm not against the code cleanup and it always welcome.
> Please also have a look at other comment.
>
> >> What is the issue with existing code that we need this patch for ?
> >>
> >
> > Hello Chaitanya,
> >
> > This is just a cleanup patch, no functional change intended.
> >
> I can see that.
> > It simply performs the initialization at declaration time, which is how we
> > usually initialize a subset of all fields.
> Absolutely not true in case nvme subsystem.
> >
> > This is also how it was originally done, but this was changed to a
> > non-standard way in order to workaround a compiler bug.
> >
> and the existing code matches the pattern present in the repo no need to
> change if it is not causing any problem.
> > Since the compiler bug is no longer relevant, we can go back to the
> > standard way of doing things.
> Is there any problem with the code now which mandates this change ?
> which I don't see any.
> >
> > Performing initialization in a uniform way makes it easier to read and
> > comprehend the code, especially for people unfamiliar with it, since
> > it follows the same pattern used in other places of the kernel.
> >
> This is absolutely not uniform way infact what you are proposing is true
> then we need to change all existing function which does not follow this
> pattern, have a look at the following list.
>
> In NVMe subsystem case there are more than 10 functions which are on the
> similar line of this function and doesn't initialize the variable at the
> time declaration.
>
> Please have a look the code :-
> nvmf_reg_read32
> nvmf_reg_read64
> nvmf_reg_write32
> nvmf_connect_admin_queue
> nvmf_connect_io_queue
> nvme_features
> nvme_toggle_streams
> nvme_get_stream_params
> nvme_user_cmd
> nvme_user_cmd64
These do not use an initializer at all, these use memset.
So at declaration time, these are not initialized at all.
Basically like:
int a;
a = 2;
I don't have any problem with the memset code pattern per se,
it is very common. Although it is weird that the nvme code
sometimes declares a "struct nvme_command c" on the stack,
and then memsets it, and sometimes uses an initializer.
Perhaps we should create a patch to unify this.
IMHO, using an initializer is more clear.
memset has to be used if the function needs to reset an
existing struct, but in none of the functions above are
we given a nvme_command that we need to reset. In each case
we declare a new nvme_command on the stack (so an initializer
makes more sense).
> nvme_identify_ctrl
> nvme_identify_ns_descs
> nvme_identify_ns_list
> nvme_identify_ns
> nvme_get_log
These used an initializer, and then also later did explict assignments,
e.g.:
struct nvme_command c = { 0 };
...
c.identify.cns = NVME_ID_CNS_CTRL;
which is basically the same as doing:
int a = 0;
a = 2;
However, I have fixed these functions in patch 1/2 in this series,
so that the values are set in the initializer, and then there is
not need for any further assignments.
Basically:
int a = 2;
>
> Last two are an exception of copy_from_user() call before initialization
> case, but we can do copy from user from caller and pass that as an
> argument if we really want to follow the declare-init pattern.
>
> In several places we don't follow this pattern when function is compact
> and it looks ugly for larger structures such as this example. this is
> exactly the reason {} used in nvme subsystem.
If this pattern of using an initializer and then doing an assignment
is the desired pattern, then you should at least remove the
/* gcc-4.4.4 (at least) has issues with initializers and anon unions */
comments from drivers/nvme/host/core.c, because then that comment is
not true, because then you want this design pattern because you like it,
not because of a gcc bug.
>
> > Just reading e.g. struct rdma_conn_param param = { }; one assumes that
> > all fields will be zero initialized.. reading futher down in the function
>
> No this is standard style and used in nvme/host/core.c everywhere
> nothing wrong with this at all, please have a look at the author.
I think that the standard style in the nvme code is to assign the
values inside the initializer when assigning values to a struct
(except when using memset), see e.g.:
nvmet_tcp_try_recv_ddgst
nvmet_tcp_try_recv_pdu
nvmet_try_send_ddgst
nvmet_rdma_init_srq
nvmet_fc_add_port
nvme_fc_parse_traddr
nvmet_copy_ns_identifier
nvme_tcp_try_send_ddgst
nvme_rdma_inv_rkey
nvme_setup_irqs
nvme_fc_create_ctrl
nvme_fc_parse_traddr
nvme_fc_signal_discovery_scan
nvme_aen_uevent
And then there are the many many
static const struct's that are assigned values using initializers.
(Which I didn't count.)
I made a small mistake in v1, so I'll send out a v2,
feel free to continue the discussion there :)
Kind regards,
Niklas
Re: [PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
On Thu, Jun 18, 2020 at 07:11:24PM +0200, Daniel Wagner wrote: > On Thu, Jun 18, 2020 at 04:32:40PM +0200, Niklas Cassel wrote: > > If, for some reason, we want to allow builds with gcc < 4.6.0 > > even though the minimum gcc version is now 4.8.0, > > Just one thing to watch out: the stable trees are still using > older version of gcc. Note sure how relevant this is though. While the AUTOSEL can be a bit annoying when autoselecting patches to backport that you didn't intend, I think that it in most cases backports fixes that has a Fixes-tag, which this doesn't. Kind regards, Niklas
Re: [PATCH 2/2] nvmet: remove workarounds for gcc bug wrt unnamed fields in initializers
On Thu, Jun 18, 2020 at 03:23:21PM +, Chaitanya Kulkarni wrote:
> On 6/18/20 7:32 AM, Niklas Cassel wrote:
> > drivers/nvme/target/rdma.c | 23 ---
> > 1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
> > index 6731e0349480..85c6ff0b0e44 100644
> > --- a/drivers/nvme/target/rdma.c
> > +++ b/drivers/nvme/target/rdma.c
> > @@ -1535,19 +1535,20 @@ static int nvmet_rdma_cm_accept(struct rdma_cm_id
> > *cm_id,
> > struct nvmet_rdma_queue *queue,
> > struct rdma_conn_param *p)
> > {
> > - struct rdma_conn_param param = { };
> > - struct nvme_rdma_cm_rep priv = { };
> > + struct rdma_conn_param param = {
> > + .rnr_retry_count = 7,
> > + .flow_control = 1,
> > + .initiator_depth = min_t(u8, p->initiator_depth,
> > + queue->dev->device->attrs.max_qp_init_rd_atom),
> > + .private_data = ,
> > + .private_data_len = sizeof(priv),
> > + };
> > + struct nvme_rdma_cm_rep priv = {
> > + .recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0),
> > + .crqsize = cpu_to_le16(queue->recv_queue_size),
> > + };
> > int ret = -ENOMEM;
> >
> > - param.rnr_retry_count = 7;
> > - param.flow_control = 1;
> > - param.initiator_depth = min_t(u8, p->initiator_depth,
> > - queue->dev->device->attrs.max_qp_init_rd_atom);
> > - param.private_data =
> > - param.private_data_len = sizeof(priv);
> > - priv.recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0);
> > - priv.crqsize = cpu_to_le16(queue->recv_queue_size);
> > -
> > ret = rdma_accept(cm_id, );
> > if (ret)
> > pr_err("rdma_accept failed (error code = %d)\n", ret);
> > -- 2.26.2
>
> What is the issue with existing code that we need this patch for ?
>
Hello Chaitanya,
This is just a cleanup patch, no functional change intended.
It simply performs the initialization at declaration time, which is how we
usually initialize a subset of all fields.
This is also how it was originally done, but this was changed to a
non-standard way in order to workaround a compiler bug.
Since the compiler bug is no longer relevant, we can go back to the
standard way of doing things.
Performing initialization in a uniform way makes it easier to read and
comprehend the code, especially for people unfamiliar with it, since
it follows the same pattern used in other places of the kernel.
Just reading e.g. struct rdma_conn_param param = { }; one assumes that
all fields will be zero initialized.. reading futher down in the function
you realize that this function actually does initialize the struct..
which causes a mental hiccup, so you need to do a mental pipeline flush
and go back and read the code from the beginning. This only happens with
patterns that deviate from the standard way of doing things.
Kind regards,
Niklas
[PATCH 2/2] nvmet: remove workarounds for gcc bug wrt unnamed fields in initializers
Workarounds for gcc issues with initializers and anon unions was first
introduced in commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build
with gcc-4.4.4").
The gcc bug in question has been fixed since gcc 4.6.0:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676
The minimum gcc version for building the kernel has been 4.6.0 since
commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6"),
and has since been updated to gcc 4.8.0 in
commit 5429ef62bcf3 ("compiler/gcc: Raise minimum GCC version for
kernel builds to 4.8").
For that reason, it should now be safe to remove these workarounds
and make the code look like it did before
commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build with gcc-4.4.4")
was introduced.
Signed-off-by: Niklas Cassel
---
If, for some reason, we want to allow builds with gcc < 4.6.0
even though the minimum gcc version is now 4.8.0,
there is another less intrusive workaround where you add an extra pair of
curly braces, see e.g. commit 6cc65be4f6f2 ("locking/qspinlock: Fix build
for anonymous union in older GCC compilers").
drivers/nvme/target/rdma.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 6731e0349480..85c6ff0b0e44 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1535,19 +1535,20 @@ static int nvmet_rdma_cm_accept(struct rdma_cm_id
*cm_id,
struct nvmet_rdma_queue *queue,
struct rdma_conn_param *p)
{
- struct rdma_conn_param param = { };
- struct nvme_rdma_cm_rep priv = { };
+ struct rdma_conn_param param = {
+ .rnr_retry_count = 7,
+ .flow_control = 1,
+ .initiator_depth = min_t(u8, p->initiator_depth,
+ queue->dev->device->attrs.max_qp_init_rd_atom),
+ .private_data = ,
+ .private_data_len = sizeof(priv),
+ };
+ struct nvme_rdma_cm_rep priv = {
+ .recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0),
+ .crqsize = cpu_to_le16(queue->recv_queue_size),
+ };
int ret = -ENOMEM;
- param.rnr_retry_count = 7;
- param.flow_control = 1;
- param.initiator_depth = min_t(u8, p->initiator_depth,
- queue->dev->device->attrs.max_qp_init_rd_atom);
- param.private_data =
- param.private_data_len = sizeof(priv);
- priv.recfmt = cpu_to_le16(NVME_RDMA_CM_FMT_1_0);
- priv.crqsize = cpu_to_le16(queue->recv_queue_size);
-
ret = rdma_accept(cm_id, );
if (ret)
pr_err("rdma_accept failed (error code = %d)\n", ret);
--
2.26.2
[PATCH 1/2] nvme: remove workarounds for gcc bug wrt unnamed fields in initializers
Workarounds for gcc issues with initializers and anon unions was first
introduced in commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build
with gcc-4.4.4").
The gcc bug in question has been fixed since gcc 4.6.0:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=10676
The minimum gcc version for building the kernel has been 4.6.0 since
commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6"),
and has since been updated to gcc 4.8.0 in
commit 5429ef62bcf3 ("compiler/gcc: Raise minimum GCC version for
kernel builds to 4.8").
For that reason, it should now be safe to remove these workarounds
and make the code look like it did before
commit e44ac588cd61 ("drivers/block/nvme-core.c: fix build with gcc-4.4.4")
was introduced.
Signed-off-by: Niklas Cassel
---
If, for some reason, we want to allow builds with gcc < 4.6.0
even though the minimum gcc version is now 4.8.0,
there is another less intrusive workaround where you add an extra pair of
curly braces, see e.g. commit 6cc65be4f6f2 ("locking/qspinlock: Fix build
for anonymous union in older GCC compilers").
drivers/nvme/host/core.c | 59 ++--
drivers/nvme/host/lightnvm.c | 32 +--
drivers/nvme/host/rdma.c | 28 -
3 files changed, 59 insertions(+), 60 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9491dbcfe81a..99059340d723 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1038,13 +1038,12 @@ static bool nvme_ctrl_limited_cns(struct nvme_ctrl
*ctrl)
static int nvme_identify_ctrl(struct nvme_ctrl *dev, struct nvme_id_ctrl **id)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.cns = NVME_ID_CNS_CTRL,
+ };
int error;
- /* gcc-4.4.4 (at least) has issues with initializers and anon unions */
- c.identify.opcode = nvme_admin_identify;
- c.identify.cns = NVME_ID_CNS_CTRL;
-
*id = kmalloc(sizeof(struct nvme_id_ctrl), GFP_KERNEL);
if (!*id)
return -ENOMEM;
@@ -1096,16 +1095,16 @@ static int nvme_process_ns_desc(struct nvme_ctrl *ctrl,
struct nvme_ns_ids *ids,
static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl, unsigned nsid,
struct nvme_ns_ids *ids)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.nsid = cpu_to_le32(nsid),
+ .identify.cns = NVME_ID_CNS_NS_DESC_LIST,
+ };
int status;
void *data;
int pos;
int len;
- c.identify.opcode = nvme_admin_identify;
- c.identify.nsid = cpu_to_le32(nsid);
- c.identify.cns = NVME_ID_CNS_NS_DESC_LIST;
-
data = kzalloc(NVME_IDENTIFY_DATA_SIZE, GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -1143,11 +1142,12 @@ static int nvme_identify_ns_descs(struct nvme_ctrl
*ctrl, unsigned nsid,
static int nvme_identify_ns_list(struct nvme_ctrl *dev, unsigned nsid, __le32
*ns_list)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.cns = NVME_ID_CNS_NS_ACTIVE_LIST,
+ .identify.nsid = cpu_to_le32(nsid),
+ };
- c.identify.opcode = nvme_admin_identify;
- c.identify.cns = NVME_ID_CNS_NS_ACTIVE_LIST;
- c.identify.nsid = cpu_to_le32(nsid);
return nvme_submit_sync_cmd(dev->admin_q, , ns_list,
NVME_IDENTIFY_DATA_SIZE);
}
@@ -1155,14 +1155,13 @@ static int nvme_identify_ns_list(struct nvme_ctrl *dev,
unsigned nsid, __le32 *n
static int nvme_identify_ns(struct nvme_ctrl *ctrl,
unsigned nsid, struct nvme_id_ns **id)
{
- struct nvme_command c = { };
+ struct nvme_command c = {
+ .identify.opcode = nvme_admin_identify,
+ .identify.nsid = cpu_to_le32(nsid),
+ .identify.cns = NVME_ID_CNS_NS,
+ };
int error;
- /* gcc-4.4.4 (at least) has issues with initializers and anon unions */
- c.identify.opcode = nvme_admin_identify;
- c.identify.nsid = cpu_to_le32(nsid);
- c.identify.cns = NVME_ID_CNS_NS;
-
*id = kmalloc(sizeof(**id), GFP_KERNEL);
if (!*id)
return -ENOMEM;
@@ -2815,17 +2814,17 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl,
struct nvme_id_ctrl *id)
int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp,
void *log, size_t size, u64 offset)
{
- struct nvme_command c = { };
u32 dwlen = nvme_bytes_to_numd(size);
-
- c.get_log_page.opcode = nvme_admin_get_log_page;
- c.get_log_page.nsid = cpu_to_le32(nsid);
- c.get_log_page.lid = log_page;
-
[PATCH 1/2] block: add max_open_zones to blk-sysfs
Add a new max_open_zones definition in the sysfs documentation.
This definition will be common for all devices utilizing the zoned block
device support in the kernel.
Export max open zones according to this new definition for NVMe Zoned
Namespace devices, ZAC ATA devices (which are treated as SCSI devices by
the kernel), and ZBC SCSI devices.
Add the new max_open_zones struct member to the request_queue, rather
than as a queue limit, since this property cannot be split across stacking
drivers.
Signed-off-by: Niklas Cassel
---
Documentation/block/queue-sysfs.rst | 7 +++
block/blk-sysfs.c | 15 +++
drivers/nvme/host/zns.c | 1 +
drivers/scsi/sd_zbc.c | 4
include/linux/blkdev.h | 20
5 files changed, 47 insertions(+)
diff --git a/Documentation/block/queue-sysfs.rst
b/Documentation/block/queue-sysfs.rst
index 6a8513af9201..f01cf8530ae4 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -117,6 +117,13 @@ Maximum number of elements in a DMA scatter/gather list
with integrity
data that will be submitted by the block layer core to the associated
block driver.
+max_open_zones (RO)
+---
+For zoned block devices (zoned attribute indicating "host-managed" or
+"host-aware"), the sum of zones belonging to any of the zone states:
+EXPLICIT OPEN or IMPLICIT OPEN, is limited by this value.
+If this value is 0, there is no limit.
+
max_sectors_kb (RW)
---
This is the maximum number of kilobytes that the block layer will allow
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 02643e149d5e..fa42961e9678 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -305,6 +305,11 @@ static ssize_t queue_nr_zones_show(struct request_queue
*q, char *page)
return queue_var_show(blk_queue_nr_zones(q), page);
}
+static ssize_t queue_max_open_zones_show(struct request_queue *q, char *page)
+{
+ return queue_var_show(queue_max_open_zones(q), page);
+}
+
static ssize_t queue_nomerges_show(struct request_queue *q, char *page)
{
return queue_var_show((blk_queue_nomerges(q) << 1) |
@@ -667,6 +672,11 @@ static struct queue_sysfs_entry queue_nr_zones_entry = {
.show = queue_nr_zones_show,
};
+static struct queue_sysfs_entry queue_max_open_zones_entry = {
+ .attr = {.name = "max_open_zones", .mode = 0444 },
+ .show = queue_max_open_zones_show,
+};
+
static struct queue_sysfs_entry queue_nomerges_entry = {
.attr = {.name = "nomerges", .mode = 0644 },
.show = queue_nomerges_show,
@@ -765,6 +775,7 @@ static struct attribute *queue_attrs[] = {
_nonrot_entry.attr,
_zoned_entry.attr,
_nr_zones_entry.attr,
+ _max_open_zones_entry.attr,
_nomerges_entry.attr,
_rq_affinity_entry.attr,
_iostats_entry.attr,
@@ -792,6 +803,10 @@ static umode_t queue_attr_visible(struct kobject *kobj,
struct attribute *attr,
(!q->mq_ops || !q->mq_ops->timeout))
return 0;
+ if (attr == _max_open_zones_entry.attr &&
+ !blk_queue_is_zoned(q))
+ return 0;
+
return attr->mode;
}
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index c08f6281b614..af156529f3b6 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -82,6 +82,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct
nvme_ns *ns,
q->limits.zoned = BLK_ZONED_HM;
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
+ blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
free_data:
kfree(id);
return status;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 183a20720da9..aa3564139b40 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -717,6 +717,10 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned
char *buf)
/* The drive satisfies the kernel restrictions: set it up */
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
+ if (sdkp->zones_max_open == U32_MAX)
+ blk_queue_max_open_zones(q, 0);
+ else
+ blk_queue_max_open_zones(q, sdkp->zones_max_open);
nr_zones = round_up(sdkp->capacity, zone_blocks) >> ilog2(zone_blocks);
/* READ16/WRITE16 is mandatory for ZBC disks */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8fd900998b4e..2f332f00501d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -520,6 +520,7 @@ struct request_queue {
unsigned intnr_zones;
unsigned long *conv_zones_bitmap;
unsigned long *seq_zones_wlock;
+ unsigned intmax_open_zones;
#endif /* CONFIG_BLK_DEV_ZON
[PATCH 2/2] block: add max_active_zones to blk-sysfs
Add a new max_active zones definition in the sysfs documentation.
This definition will be common for all devices utilizing the zoned block
device support in the kernel.
Export max_active_zones according to this new definition for NVMe Zoned
Namespace devices, ZAC ATA devices (which are treated as SCSI devices by
the kernel), and ZBC SCSI devices.
Add the new max_active_zones struct member to the request_queue, rather
than as a queue limit, since this property cannot be split across stacking
drivers.
For SCSI devices, even though max active zones is not part of the ZBC/ZAC
spec, export max_active_zones as 0, signifying "no limit".
Signed-off-by: Niklas Cassel
---
Documentation/block/queue-sysfs.rst | 7 +++
block/blk-sysfs.c | 14 +-
drivers/nvme/host/zns.c | 1 +
drivers/scsi/sd_zbc.c | 1 +
include/linux/blkdev.h | 20
5 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/Documentation/block/queue-sysfs.rst
b/Documentation/block/queue-sysfs.rst
index f01cf8530ae4..f261a5c84170 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -117,6 +117,13 @@ Maximum number of elements in a DMA scatter/gather list
with integrity
data that will be submitted by the block layer core to the associated
block driver.
+max_active_zones (RO)
+-
+For zoned block devices (zoned attribute indicating "host-managed" or
+"host-aware"), the sum of zones belonging to any of the zone states:
+EXPLICIT OPEN, IMPLICIT OPEN or CLOSED, is limited by this value.
+If this value is 0, there is no limit.
+
max_open_zones (RO)
---
For zoned block devices (zoned attribute indicating "host-managed" or
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index fa42961e9678..624bb4d85fc7 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -310,6 +310,11 @@ static ssize_t queue_max_open_zones_show(struct
request_queue *q, char *page)
return queue_var_show(queue_max_open_zones(q), page);
}
+static ssize_t queue_max_active_zones_show(struct request_queue *q, char *page)
+{
+ return queue_var_show(queue_max_active_zones(q), page);
+}
+
static ssize_t queue_nomerges_show(struct request_queue *q, char *page)
{
return queue_var_show((blk_queue_nomerges(q) << 1) |
@@ -677,6 +682,11 @@ static struct queue_sysfs_entry queue_max_open_zones_entry
= {
.show = queue_max_open_zones_show,
};
+static struct queue_sysfs_entry queue_max_active_zones_entry = {
+ .attr = {.name = "max_active_zones", .mode = 0444 },
+ .show = queue_max_active_zones_show,
+};
+
static struct queue_sysfs_entry queue_nomerges_entry = {
.attr = {.name = "nomerges", .mode = 0644 },
.show = queue_nomerges_show,
@@ -776,6 +786,7 @@ static struct attribute *queue_attrs[] = {
_zoned_entry.attr,
_nr_zones_entry.attr,
_max_open_zones_entry.attr,
+ _max_active_zones_entry.attr,
_nomerges_entry.attr,
_rq_affinity_entry.attr,
_iostats_entry.attr,
@@ -803,7 +814,8 @@ static umode_t queue_attr_visible(struct kobject *kobj,
struct attribute *attr,
(!q->mq_ops || !q->mq_ops->timeout))
return 0;
- if (attr == _max_open_zones_entry.attr &&
+ if ((attr == _max_open_zones_entry.attr ||
+attr == _max_active_zones_entry.attr) &&
!blk_queue_is_zoned(q))
return 0;
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index af156529f3b6..502070763266 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -83,6 +83,7 @@ int nvme_update_zone_info(struct gendisk *disk, struct
nvme_ns *ns,
q->limits.zoned = BLK_ZONED_HM;
blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
+ blk_queue_max_active_zones(q, le32_to_cpu(id->mar) + 1);
free_data:
kfree(id);
return status;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index aa3564139b40..d8b2c49d645b 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -721,6 +721,7 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, unsigned char
*buf)
blk_queue_max_open_zones(q, 0);
else
blk_queue_max_open_zones(q, sdkp->zones_max_open);
+ blk_queue_max_active_zones(q, 0);
nr_zones = round_up(sdkp->capacity, zone_blocks) >> ilog2(zone_blocks);
/* READ16/WRITE16 is mandatory for ZBC disks */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 2f332f00501d..3776140f8f20 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -521,6 +521,7 @@ struct request_queue {
unsigned long *conv_zones_bitmap;
[PATCH 0/2] Export max open zones and max active zones to sysfs
Export max open zones and max active zones to sysfs. This patch series depends on the Zoned Namespace Command Set series: https://lore.kernel.org/linux-nvme/20200615233424.13458-1-keith.bu...@wdc.com/ All zoned block devices in the kernel utilize the "zoned block device support" (CONFIG_BLK_DEV_ZONED). The Zoned Namespace Command Set Specification defines two different resource limits: Max Open Resources and Max Active Resources. The ZAC and ZBC standards define a MAXIMUM NUMBER OF OPEN SEQUENTIAL WRITE REQUIRED ZONES field. Since the ZNS Max Open Resources field has the same purpose as the ZAC/ZBC field, (the ZNS field is 0's based, the ZAC/ZBC field isn't), create a common "max_open_zones" definition in the sysfs documentation, and export both the ZNS field and the ZAC/ZBC field according to this new common definition. The ZNS Max Active Resources field does not have an equivalent field in ZAC/ZBC, however, since both ZAC/ZBC and ZNS utilize the "zoned block device support" in the kernel, create a "max_active_zones" definition in the sysfs documentation, similar to "max_open_zones", and export it according to this new definition. For ZAC/ZBC devices, this field will be exported as 0, meaning "no limit". Niklas Cassel (2): block: add max_open_zones to blk-sysfs block: add max_active_zones to blk-sysfs Documentation/block/queue-sysfs.rst | 14 ++ block/blk-sysfs.c | 27 +++ drivers/nvme/host/zns.c | 2 ++ drivers/scsi/sd_zbc.c | 5 include/linux/blkdev.h | 40 + 5 files changed, 88 insertions(+) -- 2.26.2
[PATCH] nvme: do not call del_gendisk() on a disk that was never added
device_add_disk() is negated by del_gendisk().
alloc_disk_node() is negated by put_disk().
In nvme_alloc_ns(), device_add_disk() is one of the last things being
called in the success case, and only void functions are being called
after this. Therefore this call should not be negated in the error path.
The superfluous call to del_gendisk() leads to the following prints:
[7.839975] kobject: '(null)' (1ff73734): is not initialized, yet
kobject_put() is being called.
[7.840865] WARNING: CPU: 2 PID: 361 at lib/kobject.c:736
kobject_put+0x70/0x120
Fixes: 33cfdc2aa696 ("nvme: enforce extended LBA format for fabrics metadata")
Signed-off-by: Niklas Cassel
---
An alternative would be to do like nvme_ns_remove(), i.e. in the error
path; check if ns->disk->flags & GENHD_FL_UP is set, and only then call
del_gendisk(). However, that seems unnecessary, since as nvme_alloc_ns()
is currently written, we know that device_add_disk() does not need to be
negated.
drivers/nvme/host/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 0585efa47d8f..c2c5bc4fb702 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3669,7 +3669,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl,
unsigned nsid)
ns->disk = disk;
if (__nvme_revalidate_disk(disk, id))
- goto out_free_disk;
+ goto out_put_disk;
if ((ctrl->quirks & NVME_QUIRK_LIGHTNVM) && id->vs[0] == 0x1) {
ret = nvme_nvm_register(ns, disk_name, node);
@@ -3696,8 +3696,6 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl,
unsigned nsid)
/* prevent double queue cleanup */
ns->disk->queue = NULL;
put_disk(ns->disk);
- out_free_disk:
- del_gendisk(ns->disk);
out_unlink_ns:
mutex_lock(>subsys->lock);
list_del_rcu(>siblings);
--
2.26.2
Re: [PATCH] nvme: prevent double free in nvme_alloc_ns() error handling
On Tue, Apr 28, 2020 at 09:06:51AM +0200, Javier González wrote:
> CAUTION: This email originated from outside of Western Digital. Do not click
> on links or open attachments unless you recognize the sender and know that
> the content is safe.
>
>
> On 27.04.2020 18:22, Niklas Cassel wrote:
> > On Mon, Apr 27, 2020 at 08:03:11PM +0200, Javier González wrote:
> > > On 27.04.2020 14:34, Niklas Cassel wrote:
> > > > When jumping to the out_put_disk label, we will call put_disk(), which
> > > > will
> > > > trigger a call to disk_release(), which calls blk_put_queue().
> > > >
> > > > Later in the cleanup code, we do blk_cleanup_queue(), which will also
> > > > call
> > > > blk_put_queue().
> > > >
> > > > Putting the queue twice is incorrect, and will generate a KASAN splat.
> > > >
> > > > Set the disk->queue pointer to NULL, before calling put_disk(), so that
> > > > the
> > > > first call to blk_put_queue() will not free the queue.
> > > >
> > > > The second call to blk_put_queue() uses another pointer to the same
> > > > queue,
> > > > so this call will still free the queue.
> > > >
> > > > Fixes: 85136c010285 ("lightnvm: simplify geometry enumeration")
> > > > Signed-off-by: Niklas Cassel
> > > > ---
> > > > drivers/nvme/host/core.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > > > index 91c1bd659947..f2adea96b04c 100644
> > > > --- a/drivers/nvme/host/core.c
> > > > +++ b/drivers/nvme/host/core.c
> > > > @@ -3642,6 +3642,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl,
> > > > unsigned nsid)
> > > >
> > > > return;
> > > > out_put_disk:
> > > > + /* prevent double queue cleanup */
> > > > + ns->disk->queue = NULL;
> > > > put_disk(ns->disk);
> > > > out_unlink_ns:
> > > > mutex_lock(>subsys->lock);
> > > > --
> > > > 2.25.3
> > > >
> > > What about delaying the assignment of ns->disk?
> > >
> > > diff --git i/drivers/nvme/host/core.c w/drivers/nvme/host/core.c
> > > index a4d8c90ee7cc..6da4a9ced945 100644
> > > --- i/drivers/nvme/host/core.c
> > > +++ w/drivers/nvme/host/core.c
> > > @@ -3541,7 +3541,6 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl,
> > > unsigned nsid)
> > > disk->queue = ns->queue;
> > > disk->flags = flags;
> > > memcpy(disk->disk_name, disk_name, DISK_NAME_LEN);
> > > - ns->disk = disk;
> > >
> > > __nvme_revalidate_disk(disk, id);
> > >
> > > @@ -3553,6 +3552,8 @@ static int nvme_alloc_ns(struct nvme_ctrl *ctrl,
> > > unsigned nsid)
> > > }
> > > }
> > >
> > > + ns->disk = disk;
> > > +
> >
> > Hello Javier!
> >
> >
> > The only case where we jump to the out_put_disk label, is if the
> > nvme_nvm_register() call failed.
> >
> > In that case, we want to undo the alloc_disk_node() operation, i.e.,
> > decrease the refcount.
> >
> > If we don't set "ns->disk = disk;" before the call to nvme_nvm_register(),
> > then, if register fails, and we jump to the put_disk(ns->disk) label,
> > ns->disk will be NULL, so the recount will not be decreased, so I assume
> > that this memory would then be a memory leak.
> >
> >
> > I think that the problem is that the block functions are a bit messy.
> > Most drivers seem to do blk_cleanup_queue() first and then do put_disk(),
> > but some drivers do it in the opposite way, so I think that we might have
> > some more use-after-free bugs in some of these drivers that do it in the
> > opposite way.
> >
>
> Hi Niklas,
>
> Yes, the out_put_disk label was introduced at the same time as the
> LightNVM entry point. We can do a better job at separating the cleanup
> functions, but as far as I can see ns->disk is not used in the LightNVM
> initialization, so delaying the initialization should be ok. Part of
> this should be also changing the out_put_disk to put_disk(disk).
Hello Javier,
If I understand correctly, you are suggesting this:
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@
Re: [PATCH] opp: of: drop incorrect lockdep_assert_held()
On Wed, Oct 23, 2019 at 11:00:05AM +0530, Viresh Kumar wrote:
> On 10-10-19, 16:00, Viresh Kumar wrote:
> > _find_opp_of_np() doesn't traverse the list of OPP tables but instead
> > just the entries within an OPP table and so only requires to lock the
> > OPP table itself.
> >
> > The lockdep_assert_held() was added there by mistake and isn't really
> > required.
> >
> > Fixes: 5d6d106fa455 ("OPP: Populate required opp tables from
> > "required-opps" property")
> > Cc: v5.0+ # v5.0+
> > Reported-by: Niklas Cassel
> > Signed-off-by: Viresh Kumar
> > ---
> > drivers/opp/of.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> > index 1813f5ad5fa2..6dc41faf74b5 100644
> > --- a/drivers/opp/of.c
> > +++ b/drivers/opp/of.c
> > @@ -77,8 +77,6 @@ static struct dev_pm_opp *_find_opp_of_np(struct
> > opp_table *opp_table,
> > {
> > struct dev_pm_opp *opp;
> >
> > - lockdep_assert_held(_table_lock);
> > -
> > mutex_lock(_table->lock);
> >
> > list_for_each_entry(opp, _table->opp_list, node) {
>
> @Niklas, any inputs from your side here would be appreciated :)
Tested-by: Niklas Cassel
After this patch, there is still a single lockdep_assert_held()
left, inside _find_table_of_opp_np(), since you kept this,
I assume that that one is still needed?
Kind regards,
Niklas
[PATCH] arm64: dts: qcom: qcs404-evb: Set vdd_apc regulator in high power mode
vdd_apc is the regulator that supplies the main CPU cluster. At sudden CPU load changes, we have noticed invalid page faults on addresses with all bits shifted, as well as on addresses with individual bits flipped. By putting the vdd_apc regulator in high power mode, the voltage drops during sudden load changes will be less severe, and we have not been able to reproduce the invalid page faults with the regulator in this mode. Signed-off-by: Niklas Cassel Suggested-by: Bjorn Andersson --- arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi index 501a7330dbc8..522d3ef72df5 100644 --- a/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi +++ b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi @@ -73,6 +73,7 @@ regulator-always-on; regulator-boot-on; regulator-name = "vdd_apc"; + regulator-initial-mode = <1>; regulator-min-microvolt = <1048000>; regulator-max-microvolt = <1384000>; }; -- 2.21.0
Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states
On Wed, Oct 02, 2019 at 11:19:50AM +0200, Niklas Cassel wrote: > On Mon, Sep 30, 2019 at 04:20:15PM -0600, Jeffrey Hugo wrote: > > Amit, the merged version of the below change causes a boot failure > > (nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly > > enough, it seems to be resolved if I remove the cpu-idle-states > > property from one of the cpu nodes. > > > > I see no issues with the msm8998 MTP. > > Hello Jeffrey, Amit, > > If the PSCI idle states work properly on the msm8998 devboard (MTP), > but causes crashes on msm8998 laptops, the only logical change is > that the PSCI firmware is different between the two devices. Since the msm8998 laptops boot using ACPI, perhaps these laptops doesn't support PSCI/have any PSCI firmware at all. Kind regards, Niklas
Re: [PATCH v2 7/9] arm64: dts: qcom: msm8998: Add PSCI cpuidle low power states
On Mon, Sep 30, 2019 at 04:20:15PM -0600, Jeffrey Hugo wrote: > Amit, the merged version of the below change causes a boot failure > (nasty hang, sometimes with RCU stalls) on the msm8998 laptops. Oddly > enough, it seems to be resolved if I remove the cpu-idle-states > property from one of the cpu nodes. > > I see no issues with the msm8998 MTP. Hello Jeffrey, Amit, If the PSCI idle states work properly on the msm8998 devboard (MTP), but causes crashes on msm8998 laptops, the only logical change is that the PSCI firmware is different between the two devices. Kind regards, Niklas
Re: [PATCH] rpmsg: glink-smem: Name the edge based on parent remoteproc
On Mon, Aug 19, 2019 at 09:16:56PM -0700, Bjorn Andersson wrote:
> Naming the glink edge device on the parent of_node short name causes
> collisions when multiple remoteproc instances with only different unit
> address are described on the platform_bus in DeviceTree.
>
> Base the edge's name on the parent remoteproc's name instead, to ensure
> that it's unique.
>
> Signed-off-by: Bjorn Andersson
> ---
> drivers/rpmsg/qcom_glink_smem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
> index 64a5ce324c7f..4238383d8685 100644
> --- a/drivers/rpmsg/qcom_glink_smem.c
> +++ b/drivers/rpmsg/qcom_glink_smem.c
> @@ -201,7 +201,7 @@ struct qcom_glink *qcom_glink_smem_register(struct device
> *parent,
> dev->parent = parent;
> dev->of_node = node;
> dev->release = qcom_glink_smem_release;
> - dev_set_name(dev, "%pOFn:%pOFn", node->parent, node);
> + dev_set_name(dev, "%s:%pOFn", dev_name(parent->parent), node);
> ret = device_register(dev);
> if (ret) {
> pr_err("failed to register glink edge\n");
> --
> 2.18.0
>
Reviewed-by: Niklas Cassel
Re: [PATCH] rpmsg: glink-smem: Name the edge based on parent remoteproc
On Mon, Aug 19, 2019 at 09:16:56PM -0700, Bjorn Andersson wrote:
> Naming the glink edge device on the parent of_node short name causes
> collisions when multiple remoteproc instances with only different unit
> address are described on the platform_bus in DeviceTree.
>
> Base the edge's name on the parent remoteproc's name instead, to ensure
> that it's unique.
>
> Signed-off-by: Bjorn Andersson
> ---
> drivers/rpmsg/qcom_glink_smem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rpmsg/qcom_glink_smem.c b/drivers/rpmsg/qcom_glink_smem.c
> index 64a5ce324c7f..4238383d8685 100644
> --- a/drivers/rpmsg/qcom_glink_smem.c
> +++ b/drivers/rpmsg/qcom_glink_smem.c
> @@ -201,7 +201,7 @@ struct qcom_glink *qcom_glink_smem_register(struct device
> *parent,
> dev->parent = parent;
> dev->of_node = node;
> dev->release = qcom_glink_smem_release;
> - dev_set_name(dev, "%pOFn:%pOFn", node->parent, node);
> + dev_set_name(dev, "%s:%pOFn", dev_name(parent->parent), node);
> ret = device_register(dev);
> if (ret) {
> pr_err("failed to register glink edge\n");
> --
> 2.18.0
>
This was sent 19 of August, then again (unchanged) on 29 of August.
Yet it is still not in linux-next.
It fixes a real issue on qcs404, so please merge :)
Kind regards,
Niklas
[PATCH] PCI: dwc: fix find_next_bit() usage
find_next_bit() takes a parameter of size long, and performs arithmetic
that assumes that the argument is of size long.
Therefore we cannot pass a u32, since this will cause find_next_bit()
to read outside the stack buffer and will produce the following print:
BUG: KASAN: stack-out-of-bounds in find_next_bit+0x38/0xb0
Fixes: 1b497e6493c4 ("PCI: dwc: Fix uninitialized variable in
dw_handle_msi_irq()")
Signed-off-by: Niklas Cassel
---
drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
b/drivers/pci/controller/dwc/pcie-designware-host.c
index d3156446ff27..45f21640c977 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -78,7 +78,8 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
{
int i, pos, irq;
- u32 val, num_ctrls;
+ unsigned long val;
+ u32 status, num_ctrls;
irqreturn_t ret = IRQ_NONE;
num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
@@ -86,14 +87,14 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
for (i = 0; i < num_ctrls; i++) {
dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS +
(i * MSI_REG_CTRL_BLOCK_SIZE),
- 4, );
- if (!val)
+ 4, );
+ if (!status)
continue;
ret = IRQ_HANDLED;
+ val = status;
pos = 0;
- while ((pos = find_next_bit((unsigned long *) ,
- MAX_MSI_IRQS_PER_CTRL,
+ while ((pos = find_next_bit(, MAX_MSI_IRQS_PER_CTRL,
pos)) != MAX_MSI_IRQS_PER_CTRL) {
irq = irq_find_mapping(pp->irq_domain,
(i * MAX_MSI_IRQS_PER_CTRL) +
--
2.21.0
[PATCH v4 06/14] dt-bindings: opp: qcom-nvmem: Support pstates provided by a power domain
Some Qualcomm SoCs have support for Core Power Reduction (CPR).
On these platforms, we need to attach to the power domain provider
providing the performance states, so that the leaky device (the CPU)
can configure the performance states (which represent different
CPU clock frequencies).
Signed-off-by: Niklas Cassel
Reviewed-by: Rob Herring
---
Changes since V3:
-In Example 2: rename the node name from cpr to power-controller,
and rename the label from cprpd to cpr.
.../bindings/opp/qcom-nvmem-cpufreq.txt | 113 +-
1 file changed, 112 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index c5ea8b90e35d..4751029b9b74 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -14,7 +14,7 @@ operating-points-v2 table when it is parsed by the OPP
framework.
Required properties:
-In 'cpus' nodes:
+In 'cpu' nodes:
- operating-points-v2: Phandle to the operating-points-v2 table to use.
In 'operating-points-v2' table:
@@ -23,6 +23,15 @@ In 'operating-points-v2' table:
Optional properties:
+In 'cpu' nodes:
+- power-domains: A phandle pointing to the PM domain specifier which provides
+ the performance states available for active state management.
+ Please refer to the power-domains bindings
+ Documentation/devicetree/bindings/power/power_domain.txt
+ and also examples below.
+- power-domain-names: Should be
+ - 'cpr' for qcs404.
+
In 'operating-points-v2' table:
- nvmem-cells: A phandle pointing to a nvmem-cells node representing the
efuse registers that has information about the
@@ -682,3 +691,105 @@ soc {
};
};
};
+
+Example 2:
+-
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x100>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU1: cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x101>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU2: cpu@102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x102>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU3: cpu@103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x103>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+ };
+
+ cpu_opp_table: cpu-opp-table {
+ compatible = "operating-points-v2-kryo-cpu";
+ opp-shared;
+
+ opp-109440 {
+ opp-hz = /bits/ 64 <109440>;
+ required-opps = <_opp1>;
+ };
+ opp-124800 {
+ opp-hz = /bits/ 64 <124800>;
+ required-opps = <_opp2>;
+ };
+ opp-140160 {
+ opp-hz = /bits/ 64 <140160>;
+ required-opps = <_opp3>;
+ };
+ };
+
+ cpr_opp_table: cpr-opp-table {
+ compatible = "operating-points-v2-qcom-level";
+
+ cpr_opp1: opp1 {
+ opp-level = <1>;
+ qcom,opp-fuse-level = <1>;
+ };
+ cpr_opp2: opp2 {
+ opp-level = <2>;
+ qcom,op
Re: [PATCH v3 06/14] dt-bindings: cpufreq: qcom-nvmem: Support pstates provided by a power domain
On Mon, Aug 19, 2019 at 10:59:36AM -0700, Stephen Boyd wrote:
> Quoting Niklas Cassel (2019-08-19 03:09:57)
> > +
> > +soc {
> > +
> > + cprpd: cpr@b018000 {
>
> Maybe node name should be 'avs' for the industry standard adaptive
> voltage scaling acronym?
I see where this is coming from, but "git grep avs" gives a single result.
Also, since the label is cprpd, it doesn't make sense to simply rename the
node name, and I don't think that avspd would be a good name, since it is
less correct.
So if you don't insist, I would prefer to leave it as it is.
>
>
> > + compatible = "qcom,qcs404-cpr", "qcom,cpr";
> > + reg = <0x0b018000 0x1000>;
> > +
> > + vdd-apc-supply = <_s3>;
> > + #power-domain-cells = <0>;
Re: [PATCH v2 10/14] dt-bindings: power: avs: Add support for CPR (Core Power Reduction)
On Fri, Aug 16, 2019 at 11:14:13PM -0700, Stephen Boyd wrote:
> Quoting Niklas Cassel (2019-07-25 03:41:38)
> > + cpr@b018000 {
> > + compatible = "qcom,qcs404-cpr", "qcom,cpr";
> > + reg = <0x0b018000 0x1000>;
> > + interrupts = <0 15 IRQ_TYPE_EDGE_RISING>;
> > + clocks = <_board>;
> > + clock-names = "ref";
> > + vdd-apc-supply = <_s3>;
> > + #power-domain-cells = <0>;
> > + operating-points-v2 = <_opp_table>;
> > + acc-syscon = <>;
> > +
> > + nvmem-cells = <_efuse_quot_offset1>,
> > + <_efuse_quot_offset2>,
> > + <_efuse_quot_offset3>,
> > + <_efuse_init_voltage1>,
> > + <_efuse_init_voltage2>,
> > + <_efuse_init_voltage3>,
> > + <_efuse_quot1>,
> > + <_efuse_quot2>,
> > + <_efuse_quot3>,
> > + <_efuse_ring1>,
> > + <_efuse_ring2>,
> > + <_efuse_ring3>,
> > + <_efuse_revision>;
> > + nvmem-cell-names = "cpr_quotient_offset1",
> > + "cpr_quotient_offset2",
> > + "cpr_quotient_offset3",
> > + "cpr_init_voltage1",
> > + "cpr_init_voltage2",
> > + "cpr_init_voltage3",
> > + "cpr_quotient1",
> > + "cpr_quotient2",
> > + "cpr_quotient3",
> > + "cpr_ring_osc1",
> > + "cpr_ring_osc2",
> > + "cpr_ring_osc3",
> > + "cpr_fuse_revision";
> > +
> > + qcom,cpr-timer-delay-us = <5000>;
> > + qcom,cpr-timer-cons-up = <0>;
> > + qcom,cpr-timer-cons-down = <2>;
> > + qcom,cpr-up-threshold = <1>;
> > + qcom,cpr-down-threshold = <3>;
> > + qcom,cpr-idle-clocks = <15>;
> > + qcom,cpr-gcnt-us = <1>;
> > + qcom,vdd-apc-step-up-limit = <1>;
> > + qcom,vdd-apc-step-down-limit = <1>;
>
> Are any of these qcom,* properties going to change for a particular SoC?
> They look like SoC config data that should just go into the driver and
> change based on the SoC compatible string.
>
Hello Stephen,
thanks a lot for your reviews.
I agree with you, will drop these properties from the dt-binding
and the driver once I respin the series.
I'm hoping to get the cpufreq part of the patch series merged this
merge window, so that the patch pile will decrease.
Kind regards,
Niklas
Re: [PATCH v3 0/8] arm64: dts: qcom: sm8150: Add SM8150 DTS
On Tue, Aug 20, 2019 at 10:53:42PM +0530, Vinod Koul wrote: > This series adds DTS for SM8150, PMIC PM8150, PM8150B, PM8150L and > the MTP for SM8150. > > Changes in v3: > - Fix copyright comment style to Linux kernel style > - Make property values all hex or decimal > - Fix patch titles and logs and make them consistent > - Fix line breaks > > Changes in v2: > - Squash patches > - Fix comments given by Stephen namely, lowercase for hext numbers, >making rpmhcc have xo_board as parent, rename pon controller to >power-on controller, make pmic nodes as disabled etc. > - removed the dependency on clk defines and use raw numbers > > > Vinod Koul (8): > arm64: dts: qcom: sm8150: Add base dts file > arm64: dts: qcom: pm8150: Add base dts file > arm64: dts: qcom: pm8150b: Add base dts file > arm64: dts: qcom: pm8150l: Add base dts file > arm64: dts: qcom: sm8150-mtp: Add base dts file > arm64: dts: qcom: sm8150-mtp: Add regulators > arm64: dts: qcom: sm8150: Add reserved-memory regions > arm64: dts: qcom: sm8150: Add apps shared nodes > > arch/arm64/boot/dts/qcom/Makefile | 1 + > arch/arm64/boot/dts/qcom/pm8150.dtsi| 97 + > arch/arm64/boot/dts/qcom/pm8150b.dtsi | 86 + > arch/arm64/boot/dts/qcom/pm8150l.dtsi | 80 > arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 378 +++ > arch/arm64/boot/dts/qcom/sm8150.dtsi| 481 > 6 files changed, 1123 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/pm8150.dtsi > create mode 100644 arch/arm64/boot/dts/qcom/pm8150b.dtsi > create mode 100644 arch/arm64/boot/dts/qcom/pm8150l.dtsi > create mode 100644 arch/arm64/boot/dts/qcom/sm8150-mtp.dts > create mode 100644 arch/arm64/boot/dts/qcom/sm8150.dtsi > > -- > 2.20.1 > Whole series is: Reviewed-by: Niklas Cassel
Re: [PATCH v2 0/8] arm64: dts: qcom: sm8150: Add SM8150 DTS
On Tue, Aug 20, 2019 at 12:12:08PM +0530, Vinod Koul wrote: > This series adds DTS for SM8150, PMIC PM8150, PM8150B, PM8150L and > the MTP for SM8150. > > Changes in v2: > - Squash patches > - Fix comments given by Stephen namely, lowercase for hext numbers, >making rpmhcc have xo_board as parent, rename pon controller to >power-on controller, make pmic nodes as disabled etc. > - removed the dependency on clk defines and use raw numbers > > Vinod Koul (8): > arm64: dts: qcom: sm8150: add base dts file > arm64: dts: qcom: pm8150: Add Base DTS file > arm64: dts: qcom: pm8150b: Add Base DTS file > arm64: dts: qcom: pm8150l: Add Base DTS file > arm64: dts: qcom: sm8150-mtp: add base dts file Use consistent naming. > arm64: dts: qcom: sm8150-mtp: Add regulators > arm64: dts: qcom: sm8150: Add reserved-memory regions > arm64: dts: qcom: sm8150: Add apps shared nodes > > arch/arm64/boot/dts/qcom/Makefile | 1 + > arch/arm64/boot/dts/qcom/pm8150.dtsi| 95 + > arch/arm64/boot/dts/qcom/pm8150b.dtsi | 84 + > arch/arm64/boot/dts/qcom/pm8150l.dtsi | 78 > arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 375 +++ > arch/arm64/boot/dts/qcom/sm8150.dtsi| 479 > 6 files changed, 1112 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/pm8150.dtsi > create mode 100644 arch/arm64/boot/dts/qcom/pm8150b.dtsi > create mode 100644 arch/arm64/boot/dts/qcom/pm8150l.dtsi > create mode 100644 arch/arm64/boot/dts/qcom/sm8150-mtp.dts > create mode 100644 arch/arm64/boot/dts/qcom/sm8150.dtsi > > -- > 2.20.1 >
Re: [PATCH v2 1/8] arm64: dts: qcom: sm8150: add base dts file
On Tue, Aug 20, 2019 at 12:12:09PM +0530, Vinod Koul wrote:
> This add base DTS file with cpu, psci, firmware, clock, tlmm and
> spmi nodes which enables boot to console
>
> Signed-off-by: Vinod Koul
> ---
> arch/arm64/boot/dts/qcom/sm8150.dtsi | 305 +++
> 1 file changed, 305 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sm8150.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> new file mode 100644
> index ..d9dc95f851b7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +// Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> +// Copyright (c) 2019, Linaro Limited
> +
> +#include
> +#include
> +#include
> +
> +/ {
> + interrupt-parent = <>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
What is the point of an empty node without a label?
Perhaps I'm missing something.
> +
> + clocks {
> + xo_board: xo-board {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <3840>;
> + clock-output-names = "xo_board";
> + };
> +
> + sleep_clk: sleep-clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32764>;
> + clock-output-names = "sleep_clk";
> + };
> + };
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + CPU0: cpu@0 {
> + device_type = "cpu";
> + compatible = "qcom,kryo485";
I don't see this compatible in
Documentation/devicetree/bindings/arm/cpus.yaml
> + reg = <0x0 0x0>;
> + enable-method = "psci";
> + next-level-cache = <_0>;
> + L2_0: l2-cache {
> + compatible = "cache";
> + next-level-cache = <_0>;
> + L3_0: l3-cache {
> + compatible = "cache";
> + };
> + };
> + };
> +
> + CPU1: cpu@100 {
> + device_type = "cpu";
> + compatible = "qcom,kryo485";
> + reg = <0x0 0x100>;
> + enable-method = "psci";
> + next-level-cache = <_100>;
> + L2_100: l2-cache {
> + compatible = "cache";
> + next-level-cache = <_0>;
> + };
> +
> + };
> +
> + CPU2: cpu@200 {
> + device_type = "cpu";
> + compatible = "qcom,kryo485";
> + reg = <0x0 0x200>;
> + enable-method = "psci";
> + next-level-cache = <_200>;
> + L2_200: l2-cache {
> + compatible = "cache";
> + next-level-cache = <_0>;
> + };
> + };
> +
> + CPU3: cpu@300 {
> + device_type = "cpu";
> + compatible = "qcom,kryo485";
> + reg = <0x0 0x300>;
> + enable-method = "psci";
> + next-level-cache = <_300>;
> + L2_300: l2-cache {
> + compatible = "cache";
> + next-level-cache = <_0>;
> + };
> + };
> +
> + CPU4: cpu@400 {
> + device_type = "cpu";
> + compatible = "qcom,kryo485";
> + reg = <0x0 0x400>;
> + enable-method = "psci";
> + next-level-cache = <_400>;
> + L2_400: l2-cache {
> + compatible = "cache";
> + next-level-cache = <_0>;
> + };
> + };
> +
> + CPU5: cpu@500 {
> + device_type = "cpu";
> + compatible = "qcom,kryo485";
> + reg = <0x0 0x500>;
> + enable-method = "psci";
> + next-level-cache = <_500>;
> + L2_500: l2-cache {
> + compatible = "cache";
> + next-level-cache = <_0>;
> + };
> + };
> +
> + CPU6: cpu@600 {
> + device_type = "cpu";
> + compatible = "qcom,kryo485";
> + reg = <0x0 0x600>;
> + enable-method = "psci";
> +
Re: [PATCH v2 3/8] arm64: dts: qcom: pm8150b: Add Base DTS file
On Tue, Aug 20, 2019 at 12:12:11PM +0530, Vinod Koul wrote:
> PMIC pm8150b is a slave pmic and this adds base DTS file for pm8150b
> with pon, adc, and gpio nodes
All of your other commit messages refers to it as power-on
instead of pon, be consistent.
>
> Signed-off-by: Vinod Koul
> ---
> arch/arm64/boot/dts/qcom/pm8150b.dtsi | 84 +++
> 1 file changed, 84 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/pm8150b.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/pm8150b.dtsi
> b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
> new file mode 100644
> index ..dfb71fb8c90a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm8150b.dtsi
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +// Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> +// Copyright (c) 2019, Linaro Limited
> +
> +#include
> +#include
> +#include
> +
> +_bus {
> + pmic@2 {
> + compatible = "qcom,pm8150b", "qcom,spmi-pmic";
> + reg = <0x2 SPMI_USID>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + power-on@800 {
> + compatible = "qcom,pm8916-pon";
> + reg = <0x0800>;
> +
> + status = "disabled";
> + };
> +
> + adc@3100 {
> + compatible = "qcom,spmi-adc5";
> + reg = <0x3100>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #io-channel-cells = <1>;
> + interrupts = <0x2 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
> +
> + status = "disabled";
> +
> + ref-gnd@0 {
> + reg = ;
> + qcom,pre-scaling = <1 1>;
> + label = "ref_gnd";
> + };
> +
> + vref-1p25@1 {
> + reg = ;
> + qcom,pre-scaling = <1 1>;
> + label = "vref_1p25";
> + };
> +
> + die-temp@6 {
> + reg = ;
> + qcom,pre-scaling = <1 1>;
> + label = "die_temp";
> + };
> +
> + chg-temp@9 {
> + reg = ;
> + qcom,pre-scaling = <1 1>;
> + label = "chg_temp";
> + };
> + };
> +
> + pm8150b_gpios: gpio@c000 {
> + compatible = "qcom,pm8150b-gpio";
> + reg = <0xc000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupts = <0x2 0xc0 0 IRQ_TYPE_NONE>,
> + <0x2 0xc1 0 IRQ_TYPE_NONE>,
> + <0x2 0xc2 0 IRQ_TYPE_NONE>,
> + <0x2 0xc3 0 IRQ_TYPE_NONE>,
> + <0x2 0xc4 0 IRQ_TYPE_NONE>,
> + <0x2 0xc5 0 IRQ_TYPE_NONE>,
> + <0x2 0xc6 0 IRQ_TYPE_NONE>,
> + <0x2 0xc7 0 IRQ_TYPE_NONE>,
> + <0x2 0xc8 0 IRQ_TYPE_NONE>,
> + <0x2 0xc9 0 IRQ_TYPE_NONE>,
> + <0x2 0xca 0 IRQ_TYPE_NONE>,
> + <0x2 0xcb 0 IRQ_TYPE_NONE>;
> + };
> + };
> +
> + pmic@3 {
> + compatible = "qcom,pm8150b", "qcom,spmi-pmic";
> + reg = <0x3 SPMI_USID>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +};
> --
> 2.20.1
>
Re: [PATCH v2 2/8] arm64: dts: qcom: pm8150: Add Base DTS file
On Tue, Aug 20, 2019 at 12:12:10PM +0530, Vinod Koul wrote:
> Add base DTS file for pm8150 along with GPIOs, power-on, rtc and vadc
> nodes
>
> Signed-off-by: Vinod Koul
> ---
> arch/arm64/boot/dts/qcom/pm8150.dtsi | 95
> 1 file changed, 95 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/pm8150.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi
> b/arch/arm64/boot/dts/qcom/pm8150.dtsi
> new file mode 100644
> index ..4a678be46d37
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +// Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> +// Copyright (c) 2019, Linaro Limited
> +
> +#include
> +#include
> +#include
> +#include
> +
> +_bus {
> + pm8150_0: pmic@0 {
> + compatible = "qcom,pm8150", "qcom,spmi-pmic";
> + reg = <0x0 SPMI_USID>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pon: power-on@800 {
> + compatible = "qcom,pm8916-pon";
> + reg = <0x0800>;
> + pwrkey {
> + compatible = "qcom,pm8941-pwrkey";
> + interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
Here you use 0 for 3rd cell
> + debounce = <15625>;
> + bias-pull-up;
> + linux,code = ;
> +
> + status = "disabled";
> + };
> + };
> +
> + pm8150_adc: adc@3100 {
> + compatible = "qcom,spmi-adc5";
> + reg = <0x3100>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #io-channel-cells = <1>;
> + interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
Here you use 0x0 for 3rd cell, be consistent.
> +
> + status = "disabled";
> +
> + ref-gnd@0 {
> + reg = ;
> + qcom,pre-scaling = <1 1>;
> + label = "ref_gnd";
> + };
> +
> + vref-1p25@1 {
> + reg = ;
> + qcom,pre-scaling = <1 1>;
> + label = "vref_1p25";
> + };
> +
> + die-temp@6 {
> + reg = ;
> + qcom,pre-scaling = <1 1>;
> + label = "die_temp";
> + };
> + };
> +
> + rtc@6000 {
> + compatible = "qcom,pm8941-rtc";
> + reg = <0x6000>;
> + reg-names = "rtc", "alarm";
> + interrupts = <0x0 0x61 0x1 IRQ_TYPE_NONE>;
> +
> + status = "disabled";
> + };
> +
> + pm8150_gpios: gpio@c000 {
> + compatible = "qcom,pm8150-gpio";
> + reg = <0xc000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupts = <0 0xc0 0 IRQ_TYPE_NONE>,
> + <0 0xc1 0 IRQ_TYPE_NONE>,
> + <0 0xc2 0 IRQ_TYPE_NONE>,
> + <0 0xc3 0 IRQ_TYPE_NONE>,
> + <0 0xc4 0 IRQ_TYPE_NONE>,
> + <0 0xc5 0 IRQ_TYPE_NONE>,
> + <0 0xc6 0 IRQ_TYPE_NONE>,
> + <0 0xc7 0 IRQ_TYPE_NONE>,
> + <0 0xc8 0 IRQ_TYPE_NONE>,
> + <0 0xc9 0 IRQ_TYPE_NONE>,
> + <0 0xca 0 IRQ_TYPE_NONE>,
> + <0 0xcb 0 IRQ_TYPE_NONE>;
> + };
> + };
> +
> + pmic@1 {
> + compatible = "qcom,pm8150", "qcom,spmi-pmic";
> + reg = <0x1 SPMI_USID>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +};
> --
> 2.20.1
>
Re: [PATCH v2 5/8] arm64: dts: qcom: sm8150-mtp: add base dts file
On Tue, Aug 20, 2019 at 12:12:13PM +0530, Vinod Koul wrote:
> This add base DTS file for sm8150-mtp and enables boot to console, adds
> tlmm reserved range, resin node, volume down key and also includes pmic
> file.
>
> Signed-off-by: Vinod Koul
> ---
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 48 +
> 2 files changed, 49 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sm8150-mtp.dts
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile
> b/arch/arm64/boot/dts/qcom/Makefile
> index 0a7e5dfce6f7..1964dacaf19b 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -12,5 +12,6 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r2.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r3.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-mtp.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += sm8150-mtp.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb
> dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> b/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> new file mode 100644
> index ..80b15f4f07c8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +// Copyright (c) 2017-2019, The Linux Foundation. All rights reserved.
> +// Copyright (c) 2019, Linaro Limited
> +
> +/dts-v1/;
> +
> +#include "sm8150.dtsi"
> +#include "pm8150.dtsi"
> +#include "pm8150b.dtsi"
> +#include "pm8150l.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. SM8150 MTP";
> + compatible = "qcom,sm8150-mtp";
> +
> + aliases {
> + serial0 =
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +};
> +
> +_id_1 {
> + status = "okay";
> +};
> +
> + {
> + pwrkey {
> + status = "okay";
> + };
> +
> + resin {
> + compatible = "qcom,pm8941-resin";
> + interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> + debounce = <15625>;
> + bias-pull-up;
> + linux,code = ;
> + };
> +};
Missing a newline here.
> + {
> + gpio-reserved-ranges = <0 4>, <126 4>;
> +};
> +
> + {
> + status = "okay";
> +};
> --
> 2.20.1
>
Re: [PATCH v2 6/8] arm64: dts: qcom: sm8150-mtp: Add regulators
On Tue, Aug 20, 2019 at 12:12:14PM +0530, Vinod Koul wrote:
> Add the regulators found in the mtp platform. This platform consists of
> pmic PM8150, PM8150L and PM8009.
Is there a reason not to squash this this patch 5/8 ?
>
> Signed-off-by: Vinod Koul
> ---
> arch/arm64/boot/dts/qcom/sm8150-mtp.dts | 327
> 1 file changed, 327 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> b/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> index 80b15f4f07c8..0513b24496f6 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8150-mtp.dts
> @@ -4,6 +4,7 @@
>
> /dts-v1/;
>
> +#include
> #include "sm8150.dtsi"
> #include "pm8150.dtsi"
> #include "pm8150b.dtsi"
> @@ -20,6 +21,332 @@
> chosen {
> stdout-path = "serial0:115200n8";
> };
> +
> + vph_pwr: vph-pwr-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "vph_pwr";
> + regulator-min-microvolt = <370>;
> + regulator-max-microvolt = <370>;
> + };
> +
> + /*
> + * Apparently RPMh does not provide support for PM8150 S4 because it
> + * is always-on; model it as a fixed regulator.
> + */
> + vreg_s4a_1p8: pm8150-s4 {
> + compatible = "regulator-fixed";
> + regulator-name = "vreg_s4a_1p8";
> +
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> +
> + regulator-always-on;
> + regulator-boot-on;
> +
> + vin-supply = <_pwr>;
> + };
> +};
> +
> +_rsc {
> + pm8150-rpmh-regulators {
> + compatible = "qcom,pm8150-rpmh-regulators";
> + qcom,pmic-id = "a";
> +
> + vdd-s1-supply = <_pwr>;
> + vdd-s2-supply = <_pwr>;
> + vdd-s3-supply = <_pwr>;
> + vdd-s4-supply = <_pwr>;
> + vdd-s5-supply = <_pwr>;
> + vdd-s6-supply = <_pwr>;
> + vdd-s7-supply = <_pwr>;
> + vdd-s8-supply = <_pwr>;
> + vdd-s9-supply = <_pwr>;
> + vdd-s10-supply = <_pwr>;
> +
> + vdd-l1-l8-l11-supply = <_s6a_0p9>;
> + vdd-l2-l10-supply = <_bob>;
> + vdd-l3-l4-l5-l18-supply = <_s6a_0p9>;
> + vdd-l6-l9-supply = <_s8c_1p3>;
> + vdd-l7-l12-l14-l15-supply = <_s5a_2p0>;
> + vdd-l13-l16-l17-supply = <_bob>;
> +
> + vreg_s5a_2p0: smps5 {
> + regulator-min-microvolt = <1904000>;
> + regulator-max-microvolt = <200>;
> + };
> +
> + vreg_s6a_0p9: smps6 {
> + regulator-min-microvolt = <92>;
> + regulator-max-microvolt = <1128000>;
> + };
> +
> + vdda_wcss_pll:
> + vreg_l1a_0p75: ldo1 {
> + regulator-min-microvolt = <752000>;
> + regulator-max-microvolt = <752000>;
> + regulator-initial-mode = ;
> + };
> +
> + vdd_pdphy:
> + vdda_usb_hs_3p1:
> + vreg_l2a_3p1: ldo2 {
> + regulator-min-microvolt = <3072000>;
> + regulator-max-microvolt = <3072000>;
> + regulator-initial-mode = ;
> + };
> +
> + vreg_l3a_0p8: ldo3 {
> + regulator-min-microvolt = <48>;
> + regulator-max-microvolt = <932000>;
> + regulator-initial-mode = ;
> + };
> +
> + vdd_usb_hs_core:
> + vdda_csi_0_0p9:
> + vdda_csi_1_0p9:
> + vdda_csi_2_0p9:
> + vdda_csi_3_0p9:
> + vdda_dsi_0_0p9:
> + vdda_dsi_1_0p9:
> + vdda_dsi_0_pll_0p9:
> + vdda_dsi_1_pll_0p9:
> + vdda_pcie_1ln_core:
> + vdda_pcie_2ln_core:
> + vdda_pll_hv_cc_ebi01:
> + vdda_pll_hv_cc_ebi23:
> + vdda_qrefs_0p875_5:
> + vdda_sp_sensor:
> + vdda_ufs_2ln_core_1:
> + vdda_ufs_2ln_core_2:
> + vdda_usb_ss_dp_core_1:
> + vdda_usb_ss_dp_core_2:
> + vdda_qlink_lv:
> + vdda_qlink_lv_ck:
> + vreg_l5a_0p875: ldo5 {
> + regulator-min-microvolt = <88>;
> + regulator-max-microvolt = <88>;
> + regulator-initial-mode = ;
> + };
> +
> + vreg_l6a_1p2: ldo6 {
> + regulator-min-microvolt = <120>;
> + regulator-max-microvolt = <120>;
> + regulator-initial-mode = ;
> + };
> +
> + vreg_l7a_1p8: ldo7 {
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> +
Re: [PATCH v2 7/8] arm64: dts: qcom: sm8150: Add reserved-memory regions
On Tue, Aug 20, 2019 at 12:12:15PM +0530, Vinod Koul wrote:
> Add the reserved memory regions in SM8150
Is there a reason not to squash this this patch 1/8 ?
>
> Signed-off-by: Vinod Koul
> ---
> arch/arm64/boot/dts/qcom/sm8150.dtsi | 111 +++
> 1 file changed, 111 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> index d9dc95f851b7..8bf4b4c17ae0 100644
> --- a/arch/arm64/boot/dts/qcom/sm8150.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8150.dtsi
> @@ -153,6 +153,117 @@
> method = "smc";
> };
>
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + hyp_mem: memory@8570 {
> + reg = <0x0 0x8570 0x0 0x60>;
> + no-map;
> + };
> +
> + xbl_mem: memory@85d0 {
> + reg = <0x0 0x85d0 0x0 0x14>;
> + no-map;
> + };
> +
> + aop_mem: memory@85f0 {
> + reg = <0x0 0x85f0 0x0 0x2>;
> + no-map;
> + };
> +
> + aop_cmd_db: memory@85f2 {
> + compatible = "qcom,cmd-db";
> + reg = <0x0 0x85f2 0x0 0x2>;
> + no-map;
> + };
> +
> + smem_mem: memory@8600 {
> + reg = <0x0 0x8600 0x0 0x20>;
> + no-map;
> + };
> +
> + tz_mem: memory@8620 {
> + reg = <0x0 0x8620 0x0 0x390>;
> + no-map;
> + };
> +
> + rmtfs_mem: memory@89b0 {
> + compatible = "qcom,rmtfs-mem";
> + reg = <0x0 0x89b0 0x0 0x20>;
> + no-map;
> +
> + qcom,client-id = <1>;
> + qcom,vmid = <15>;
> + };
> +
> + camera_mem: memory@8b70 {
> + reg = <0x0 0x8b70 0x0 0x50>;
> + no-map;
> + };
> +
> + wlan_mem: memory@8bc0 {
> + reg = <0x0 0x8bc0 0x0 0x18>;
> + no-map;
> + };
> +
> + npu_mem: memory@8bd8 {
> + reg = <0x0 0x8bd8 0x0 0x8>;
> + no-map;
> + };
> +
> + adsp_mem: memory@8be0 {
> + reg = <0x0 0x8be0 0x0 0x1a0>;
> + no-map;
> + };
> +
> + mpss_mem: memory@8d80 {
> + reg = <0x0 0x8d80 0x0 0x960>;
> + no-map;
> + };
> +
> + venus_mem: memory@96e0 {
> + reg = <0x0 0x96e0 0x0 0x50>;
> + no-map;
> + };
> +
> + slpi_mem: memory@9730 {
> + reg = <0x0 0x9730 0x0 0x140>;
> + no-map;
> + };
> +
> + ipa_fw_mem: memory@9870 {
> + reg = <0x0 0x9870 0x0 0x1>;
> + no-map;
> + };
> +
> + ipa_gsi_mem: memory@9871 {
> + reg = <0x0 0x9871 0x0 0x5000>;
> + no-map;
> + };
> +
> + gpu_mem: memory@98715000 {
> + reg = <0x0 0x98715000 0x0 0x2000>;
> + no-map;
> + };
> +
> + spss_mem: memory@9880 {
> + reg = <0x0 0x9880 0x0 0x10>;
> + no-map;
> + };
> +
> + cdsp_mem: memory@9890 {
> + reg = <0x0 0x9890 0x0 0x140>;
> + no-map;
> + };
> +
> + qseecom_mem: memory@9e40 {
> + reg = <0 0x9e40 0 0x140>;
> + no-map;
> + };
> + };
> +
> soc: soc@0 {
> #address-cells = <1>;
> #size-cells = <1>;
> --
> 2.20.1
>
[PATCH v3 09/14] dt-bindings: opp: Add qcom-opp bindings with properties needed for CPR
Add qcom-opp bindings with properties needed for Core Power Reduction (CPR). CPR is included in a great variety of Qualcomm SoCs, e.g. msm8916 and msm8996. CPR was first introduced in msm8974. Co-developed-by: Jorge Ramirez-Ortiz Signed-off-by: Jorge Ramirez-Ortiz Signed-off-by: Niklas Cassel --- Changes since V2: qcom,opp-fuse-level is really a required property and not an optional property, so properly define it as such. .../devicetree/bindings/opp/qcom-opp.txt | 19 +++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt new file mode 100644 index ..32eb0793c7e6 --- /dev/null +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt @@ -0,0 +1,19 @@ +Qualcomm OPP bindings to describe OPP nodes + +The bindings are based on top of the operating-points-v2 bindings +described in Documentation/devicetree/bindings/opp/opp.txt +Additional properties are described below. + +* OPP Table Node + +Required properties: +- compatible: Allow OPPs to express their compatibility. It should be: + "operating-points-v2-qcom-level" + +* OPP Node + +Required properties: +- qcom,opp-fuse-level: A positive value representing the fuse corner/level + associated with this OPP node. Sometimes several corners/levels shares + a certain fuse corner/level. A fuse corner/level contains e.g. ref uV, + min uV, and max uV. -- 2.21.0
[PATCH v3 06/14] dt-bindings: cpufreq: qcom-nvmem: Support pstates provided by a power domain
Some Qualcomm SoCs have support for Core Power Reduction (CPR).
On these platforms, we need to attach to the power domain provider
providing the performance states, so that the leaky device (the CPU)
can configure the performance states (which represent different
CPU clock frequencies).
Signed-off-by: Niklas Cassel
Reviewed-by: Rob Herring
---
Changes since V2:
-Picked up Rob's Reviewed-by on V2.
-As Rob pointed out in V1, it should be
"In 'cpu' nodes" and not "In 'cpus' nodes".
-In Example 2: include the qcom,opp-fuse-level property rather than "...",
since Rob pointed out in the review of V1 of "dt-bindings: opp: Add
qcom-opp bindings with properties needed for CPR", that this property was
missing in this patch.
.../bindings/opp/qcom-nvmem-cpufreq.txt | 113 +-
1 file changed, 112 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index c5ea8b90e35d..1e6261570f3e 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -14,7 +14,7 @@ operating-points-v2 table when it is parsed by the OPP
framework.
Required properties:
-In 'cpus' nodes:
+In 'cpu' nodes:
- operating-points-v2: Phandle to the operating-points-v2 table to use.
In 'operating-points-v2' table:
@@ -23,6 +23,15 @@ In 'operating-points-v2' table:
Optional properties:
+In 'cpu' nodes:
+- power-domains: A phandle pointing to the PM domain specifier which provides
+ the performance states available for active state management.
+ Please refer to the power-domains bindings
+ Documentation/devicetree/bindings/power/power_domain.txt
+ and also examples below.
+- power-domain-names: Should be
+ - 'cpr' for qcs404.
+
In 'operating-points-v2' table:
- nvmem-cells: A phandle pointing to a nvmem-cells node representing the
efuse registers that has information about the
@@ -682,3 +691,105 @@ soc {
};
};
};
+
+Example 2:
+-
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x100>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU1: cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x101>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU2: cpu@102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x102>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU3: cpu@103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x103>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+ };
+
+ cpu_opp_table: cpu-opp-table {
+ compatible = "operating-points-v2-kryo-cpu";
+ opp-shared;
+
+ opp-109440 {
+ opp-hz = /bits/ 64 <109440>;
+ required-opps = <_opp1>;
+ };
+ opp-124800 {
+ opp-hz = /bits/ 64 <124800>;
+ required-opps = <_opp2>;
+ };
+ opp-140160 {
+ opp-hz = /bits/ 64 <140160>;
+ required-opps = <_opp3>;
+ };
+ };
+
+ cpr_opp_table: cpr-opp-table {
+ c
Re: [PATCH] arm64: dts: qcom: qcs404-evb: Mark WCSS clocks protected
On Tue, Aug 13, 2019 at 08:09:42PM -0700, Bjorn Andersson wrote:
> '7d0c76bdf227 ("clk: qcom: Add WCSS gcc clock control for QCS404")'
> introduces two new clocks to gcc. These are not used before
> clk_disable_unused() and as such the clock framework tries to disable
> them.
>
> But on the EVB these registers are only accessible through TrustZone, so
> these clocks must be marked as "protected" to prevent the clock code
> from touching them.
>
> Numerical values are used as the constants are not yet available in a
> common tree.
>
> Reported-by: Mark Brown
> Reported-by: Niklas Cassel
> Signed-off-by: Bjorn Andersson
> ---
> arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> index 2289b01ee9f0..501a7330dbc8 100644
> --- a/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs404-evb.dtsi
> @@ -61,7 +61,9 @@
> protected-clocks = ,
> ,
> ,
> -;
> +,
> +<141>, /* GCC_WCSS_Q6_AHB_CLK */
> + <142>; /* GCC_WCSS_Q6_AXIM_CLK */
> };
>
> _spmi_regulators {
> --
> 2.18.0
>
Reviewed-by: Niklas Cassel
Re: [PATCH 00/15] thermal: qcom: tsens: Add interrupt support
On Mon, Jul 29, 2019 at 03:20:11PM +0530, Amit Kucheria wrote: > On Mon, Jul 29, 2019 at 3:03 PM Luca Weiss wrote: > > > > On Montag, 29. Juli 2019 11:07:35 CEST Brian Masney wrote: > > > On Sat, Jul 27, 2019 at 12:58:54PM +0530, Amit Kucheria wrote: > > > > On Fri, Jul 26, 2019 at 4:59 PM Brian Masney > > > > wrote: > > > > > On Fri, Jul 26, 2019 at 04:40:16PM +0530, Amit Kucheria wrote: > > > > > > How well does cpufreq work on 8974? I haven't looked at it yet but > > > > > > we'll need it for thermal throttling. > > > > > > > > > > I'm not sure how to tell if the frequency is dynamically changed > > > > > during > > > > > runtime on arm. x86-64 shows this information in /proc/cpuinfo. Here's > > > > > > > > > the /proc/cpuinfo on the Nexus 5: > > > > Nah. /proc/cpuinfo won't show what we need. > > > > > > > > Try the following: > > > > > > > > $ grep "" /sys/devices/system/cpu/cpufreq/policy?/* > > > > > > > > More specifically, the following files have the information you need. > > > > Run watch -n1 on them. > > > > > > > > $ grep "" /sys/devices/system/cpu/cpufreq/policy?/scaling_*_freq > > > > > > There's no cpufreq directory on msm8974: > > > > > > # ls -1 /sys/devices/system/cpu/ > > > cpu0 > > > cpu1 > > > cpu2 > > > cpu3 > > > cpuidle > > > hotplug > > > isolated > > > kernel_max > > > modalias > > > offline > > > online > > > possible > > > power > > > present > > > smt > > > uevent > > > > > > I'm using qcom_defconfig. > > > > > > Brian > > > > Hi Brian, > > cpufreq isn't supported on msm8974 yet. > > I have these patches [0] in my tree but I'm not sure they work correctly, > > but I haven't tested much with them. Feel free to try them on hammerhead. > > > > Luca > > > > [0] > > https://github.com/z3ntu/linux/compare/b0917f53ada0e929896a094b451219cd8091366e...6459ca6aff498c9d12acd35709b4903effc4c3f8 > > Niklas is working on refactoring some of the Krait code[1]. I'm not > sure if he looked at 8974 directly as part of the refactor adding him > here to get a better sense of the state of cpufreq on 8974. Hello, I took and cleaned up Sricharans commit "cpufreq: qcom: Re-organise kryo cpufreq to use it for other nvmem based qcom socs" from his Krait cpufreq series. The commit renames and refactors the Kryo cpufreq driver. This commit is now in linux-next: https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=cpufreq/arm/linux-next=106b976debd36b0e61847769f8edd71bfea56ed7 I also added Qualcomm A53 support to this driver. However, Krait CPUs are different from both Kryo and Qualcomm A53, so you will need to take Sricharans patch series and rebase it on top of linux-next. Kind regards, Niklas > > [1] > https://lore.kernel.org/linux-arm-msm/20190726080823.xwhxagv5iuhudmic@vireshk-i7/T/#t
[PATCH v2 13/14] arm64: defconfig: enable CONFIG_QCOM_CPR
Enable CONFIG_QCOM_CPR. Signed-off-by: Niklas Cassel --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 04b7fb26a942..3e7618818250 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -420,6 +420,7 @@ CONFIG_GPIO_PCA953X_IRQ=y CONFIG_GPIO_MAX77620=y CONFIG_POWER_AVS=y CONFIG_ROCKCHIP_IODOMAIN=y +CONFIG_QCOM_CPR=y CONFIG_POWER_RESET_MSM=y CONFIG_POWER_RESET_XGENE=y CONFIG_POWER_RESET_SYSCON=y -- 2.21.0
[PATCH v2 12/14] arm64: dts: qcom: qcs404: Add CPR and populate OPP table
Add CPR and populate OPP table.
Co-developed-by: Jorge Ramirez-Ortiz
Signed-off-by: Jorge Ramirez-Ortiz
Signed-off-by: Niklas Cassel
---
Changes since V1:
-Removed opp-hz from CPR OPP table.
arch/arm64/boot/dts/qcom/qcs404.dtsi | 142 +--
1 file changed, 134 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi
b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index ff9198740431..5519422b762d 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -38,7 +38,8 @@
#cooling-cells = <2>;
clocks = <_glb>;
operating-points-v2 = <_opp_table>;
- cpu-supply = <_s3>;
+ power-domains = <>;
+ power-domain-names = "cpr";
};
CPU1: cpu@101 {
@@ -51,7 +52,8 @@
#cooling-cells = <2>;
clocks = <_glb>;
operating-points-v2 = <_opp_table>;
- cpu-supply = <_s3>;
+ power-domains = <>;
+ power-domain-names = "cpr";
};
CPU2: cpu@102 {
@@ -64,7 +66,8 @@
#cooling-cells = <2>;
clocks = <_glb>;
operating-points-v2 = <_opp_table>;
- cpu-supply = <_s3>;
+ power-domains = <>;
+ power-domain-names = "cpr";
};
CPU3: cpu@103 {
@@ -77,7 +80,8 @@
#cooling-cells = <2>;
clocks = <_glb>;
operating-points-v2 = <_opp_table>;
- cpu-supply = <_s3>;
+ power-domains = <>;
+ power-domain-names = "cpr";
};
L2_0: l2-cache {
@@ -101,20 +105,37 @@
};
cpu_opp_table: cpu-opp-table {
- compatible = "operating-points-v2";
+ compatible = "operating-points-v2-kryo-cpu";
opp-shared;
opp-109440 {
opp-hz = /bits/ 64 <109440>;
- opp-microvolt = <1224000 1224000 1224000>;
+ required-opps = <_opp1>;
};
opp-124800 {
opp-hz = /bits/ 64 <124800>;
- opp-microvolt = <1288000 1288000 1288000>;
+ required-opps = <_opp2>;
};
opp-140160 {
opp-hz = /bits/ 64 <140160>;
- opp-microvolt = <1384000 1384000 1384000>;
+ required-opps = <_opp3>;
+ };
+ };
+
+ cpr_opp_table: cpr-opp-table {
+ compatible = "operating-points-v2-qcom-level";
+
+ cpr_opp1: opp1 {
+ opp-level = <1>;
+ qcom,opp-fuse-level = <1>;
+ };
+ cpr_opp2: opp2 {
+ opp-level = <2>;
+ qcom,opp-fuse-level = <2>;
+ };
+ cpr_opp3: opp3 {
+ opp-level = <3>;
+ qcom,opp-fuse-level = <3>;
};
};
@@ -294,6 +315,62 @@
tsens_caldata: caldata@d0 {
reg = <0x1f8 0x14>;
};
+ cpr_efuse_speedbin: speedbin@13c {
+ reg = <0x13c 0x4>;
+ bits = <2 3>;
+ };
+ cpr_efuse_quot_offset1: qoffset1@231 {
+ reg = <0x231 0x4>;
+ bits = <4 7>;
+ };
+ cpr_efuse_quot_offset2: qoffset2@232 {
+ reg = <0x232 0x4>;
+ bits = <3 7>;
+ };
+ cpr_efuse_quot_offset3: qoffset3@233 {
+ reg = <0x233 0x4>;
+ bits = <2 7>;
+ };
+ cpr_efuse_init_voltage1: ivoltage1@229 {
+ reg = <0x229 0x4>;
+ bits = <4 6>;
+ };
+ cpr_efuse_init_voltage2: ivoltage2@22a {
+ reg = <0x22a 0x4>;
+
[PATCH v2 09/14] dt-bindings: opp: Add qcom-opp bindings with properties needed for CPR
Add qcom-opp bindings with properties needed for Core Power Reduction (CPR). CPR is included in a great variety of Qualcomm SoCs, e.g. msm8916 and msm8996. CPR was first introduced in msm8974. Co-developed-by: Jorge Ramirez-Ortiz Signed-off-by: Jorge Ramirez-Ortiz Signed-off-by: Niklas Cassel --- .../devicetree/bindings/opp/qcom-opp.txt | 19 +++ 1 file changed, 19 insertions(+) create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt new file mode 100644 index ..f204685d029c --- /dev/null +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt @@ -0,0 +1,19 @@ +Qualcomm OPP bindings to describe OPP nodes + +The bindings are based on top of the operating-points-v2 bindings +described in Documentation/devicetree/bindings/opp/opp.txt +Additional properties are described below. + +* OPP Table Node + +Required properties: +- compatible: Allow OPPs to express their compatibility. It should be: + "operating-points-v2-qcom-level" + +* OPP Node + +Optional properties: +- qcom,opp-fuse-level: A positive value representing the fuse corner/level + associated with this OPP node. Sometimes several corners/levels shares + a certain fuse corner/level. A fuse corner/level contains e.g. ref uV, + min uV, and max uV. -- 2.21.0
[PATCH v2 10/14] dt-bindings: power: avs: Add support for CPR (Core Power Reduction)
Add DT bindings to describe the CPR HW found on certain Qualcomm SoCs.
Co-developed-by: Jorge Ramirez-Ortiz
Signed-off-by: Jorge Ramirez-Ortiz
Signed-off-by: Niklas Cassel
Reviewed-by: Rob Herring
---
Changes since V1:
-Picked up tags.
.../bindings/power/avs/qcom,cpr.txt | 193 ++
1 file changed, 193 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
diff --git a/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
new file mode 100644
index ..93be67fa8f38
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
@@ -0,0 +1,193 @@
+QCOM CPR (Core Power Reduction)
+
+CPR (Core Power Reduction) is a technology to reduce core power on a CPU
+or other device. Each OPP of a device corresponds to a "corner" that has
+a range of valid voltages for a particular frequency. While the device is
+running at a particular frequency, CPR monitors dynamic factors such as
+temperature, etc. and suggests adjustments to the voltage to save power
+and meet silicon characteristic requirements.
+
+- compatible:
+ Usage: required
+ Value type:
+ Definition: should be "qcom,qcs404-cpr", "qcom,cpr" for qcs404
+
+- reg:
+ Usage: required
+ Value type:
+ Definition: base address and size of the rbcpr register region
+
+- interrupts:
+ Usage: required
+ Value type:
+ Definition: should specify the CPR interrupt
+
+- clocks:
+ Usage: required
+ Value type:
+ Definition: phandle to the reference clock
+
+- clock-names:
+ Usage: required
+ Value type:
+ Definition: must be "ref"
+
+- vdd-apc-supply:
+ Usage: required
+ Value type:
+ Definition: phandle to the vdd-apc-supply regulator
+
+- #power-domain-cells:
+ Usage: required
+ Value type:
+ Definition: should be 0
+
+- operating-points-v2:
+ Usage: required
+ Value type:
+ Definition: A phandle to the OPP table containing the
+ performance states supported by the CPR
+ power domain
+
+- acc-syscon:
+ Usage: optional
+ Value type:
+ Definition: phandle to syscon for writing ACC settings
+
+- nvmem-cells:
+ Usage: required
+ Value type:
+ Definition: phandle to nvmem cells containing the data
+ that makes up a fuse corner, for each fuse corner.
+ As well as the CPR fuse revision.
+
+- nvmem-cell-names:
+ Usage: required
+ Value type:
+ Definition: should be "cpr_quotient_offset1", "cpr_quotient_offset2",
+ "cpr_quotient_offset3", "cpr_init_voltage1",
+ "cpr_init_voltage2", "cpr_init_voltage3", "cpr_quotient1",
+ "cpr_quotient2", "cpr_quotient3", "cpr_ring_osc1",
+ "cpr_ring_osc2", "cpr_ring_osc3", "cpr_fuse_revision"
+ for qcs404.
+
+- qcom,cpr-timer-delay-us:
+ Usage: required
+ Value type:
+ Definition: delay in uS for the timer interval
+
+- qcom,cpr-timer-cons-up:
+ Usage: required
+ Value type:
+ Definition: Consecutive number of timer intervals, or units of
+ qcom,cpr-timer-delay-us, that occur before issuing an up
+ interrupt
+
+- qcom,cpr-timer-cons-down:
+ Usage: required
+ Value type:
+ Definition: Consecutive number of timer intervals, or units of
+ qcom,cpr-timer-delay-us, that occur before issuing a down
+ interrupt
+
+- qcom,cpr-up-threshold:
+ Usage: optional
+ Value type:
+ Definition: The threshold for CPR to issue interrupt when error_steps
+ is greater than it when stepping up
+
+- qcom,cpr-down-threshold:
+ Usage: optional
+ Value type:
+ Definition: The threshold for CPR to issue interrupt when error_steps
+ is greater than it when stepping down
+
+- qcom,cpr-idle-clocks:
+ Usage: optional
+ Value type:
+ Definition: Idle clock cycles ring oscillator can be in
+
+- qcom,cpr-gcnt-us:
+ Usage: required
+ Value type:
+ Definition: The time for gate count in uS
+
+- qcom,vdd-apc-step-up-limit:
+ Usage: required
+ Value type:
+ Definition: Limit of number of vdd-apc-supply regulator steps for
+ scaling up
+
+- qcom,vdd-apc-step-down-limit:
+ Usage: required
+ Value type:
+ Definition: Limit of number of vdd-apc-supply regulator steps for
+ scaling down
+
+Example:
+
+ cpr_opp_table: cpr-opp-table {
+ compatible = "operating-po
[PATCH v2 14/14] arm64: defconfig: enable CONFIG_ARM_QCOM_CPUFREQ_NVMEM
Enable CONFIG_ARM_QCOM_CPUFREQ_NVMEM. Signed-off-by: Niklas Cassel --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 3e7618818250..9b0cc49f5fe8 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -84,6 +84,7 @@ CONFIG_ACPI_CPPC_CPUFREQ=m CONFIG_ARM_ARMADA_37XX_CPUFREQ=y CONFIG_ARM_SCPI_CPUFREQ=y CONFIG_ARM_IMX_CPUFREQ_DT=m +CONFIG_ARM_QCOM_CPUFREQ_NVMEM=y CONFIG_ARM_RASPBERRYPI_CPUFREQ=m CONFIG_ARM_TEGRA186_CPUFREQ=y CONFIG_ARM_SCPI_PROTOCOL=y -- 2.21.0
[PATCH v2 08/14] cpufreq: Add qcs404 to cpufreq-dt-platdev blacklist
From: Jorge Ramirez-Ortiz
Add qcs404 to cpufreq-dt-platdev blacklist.
Signed-off-by: Jorge Ramirez-Ortiz
Co-developed-by: Niklas Cassel
Signed-off-by: Niklas Cassel
---
drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c
b/drivers/cpufreq/cpufreq-dt-platdev.c
index f9444ddd35ab..ec2057ddb4f4 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -125,6 +125,7 @@ static const struct of_device_id blacklist[] __initconst = {
{ .compatible = "qcom,apq8096", },
{ .compatible = "qcom,msm8996", },
+ { .compatible = "qcom,qcs404", },
{ .compatible = "st,stih407", },
{ .compatible = "st,stih410", },
--
2.21.0
[PATCH v2 11/14] power: avs: Add support for CPR (Core Power Reduction)
CPR (Core Power Reduction) is a technology that reduces core power on a CPU or other device. It reads voltage settings in efuse from product test process as initial settings. Each OPP corresponds to a "corner" that has a range of valid voltages for a particular frequency. While the device is running at a particular frequency, CPR monitors dynamic factors such as temperature, etc. and adjusts the voltage for that frequency accordingly to save power and meet silicon characteristic requirements. This driver is based on an RFC by Stephen Boyd[1], which in turn is based on work by others on codeaurora.org[2]. [1] https://lkml.org/lkml/2015/9/18/833 [2] https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/regulator/cpr-regulator.c?h=msm-3.10 Co-developed-by: Jorge Ramirez-Ortiz Signed-off-by: Jorge Ramirez-Ortiz Signed-off-by: Niklas Cassel --- Changes since V1: -Implemented cpr_get_opp_hz_for_req() that uses dev_pm_opp_find_level_exact(), in order to get the opp-hz from the OPP's "required-opps" property, rather than duplicating the opp-hz property in both the CPR OPP table and the CPU OPP table. MAINTAINERS |9 + drivers/power/avs/Kconfig| 15 + drivers/power/avs/Makefile |1 + drivers/power/avs/qcom-cpr.c | 1885 ++ 4 files changed, 1910 insertions(+) create mode 100644 drivers/power/avs/qcom-cpr.c diff --git a/MAINTAINERS b/MAINTAINERS index d6b42e2413e4..a8177ff9f080 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13323,6 +13323,15 @@ S: Maintained F: Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt F: drivers/cpufreq/qcom-cpufreq-nvmem.c +QUALCOMM CORE POWER REDUCTION (CPR) AVS DRIVER +M: Niklas Cassel +M: Jorge Ramirez-Ortiz +L: linux...@vger.kernel.org +L: linux-arm-...@vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/power/avs/qcom,cpr.txt +F: drivers/power/avs/qcom-cpr.c + QUALCOMM EMAC GIGABIT ETHERNET DRIVER M: Timur Tabi L: net...@vger.kernel.org diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig index b5a217b828dc..4d4d742b3c6f 100644 --- a/drivers/power/avs/Kconfig +++ b/drivers/power/avs/Kconfig @@ -12,6 +12,21 @@ menuconfig POWER_AVS Say Y here to enable Adaptive Voltage Scaling class support. +config QCOM_CPR + tristate "QCOM Core Power Reduction (CPR) support" + depends on POWER_AVS + select PM_OPP + help + Say Y here to enable support for the CPR hardware found on Qualcomm + SoCs like MSM8916. + + This driver populates CPU OPPs tables and makes adjustments to the + tables based on feedback from the CPR hardware. If you want to do + CPUfrequency scaling say Y here. + + To compile this driver as a module, choose M here: the module will + be called qcom-cpr + config ROCKCHIP_IODOMAIN tristate "Rockchip IO domain support" depends on POWER_AVS && ARCH_ROCKCHIP && OF diff --git a/drivers/power/avs/Makefile b/drivers/power/avs/Makefile index a1b8cd453f19..8cd17e6660ee 100644 --- a/drivers/power/avs/Makefile +++ b/drivers/power/avs/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0-only obj-$(CONFIG_POWER_AVS_OMAP) += smartreflex.o obj-$(CONFIG_ROCKCHIP_IODOMAIN)+= rockchip-io-domain.o +obj-$(CONFIG_QCOM_CPR) += qcom-cpr.o diff --git a/drivers/power/avs/qcom-cpr.c b/drivers/power/avs/qcom-cpr.c new file mode 100644 index ..d6bce2832589 --- /dev/null +++ b/drivers/power/avs/qcom-cpr.c @@ -0,0 +1,1885 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved. + * Copyright (c) 2019, Linaro Limited + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* Register Offsets for RB-CPR and Bit Definitions */ + +/* RBCPR Version Register */ +#define REG_RBCPR_VERSION 0 +#define RBCPR_VER_20x02 +#define FLAGS_IGNORE_1ST_IRQ_STATUSBIT(0) + +/* RBCPR Gate Count and Target Registers */ +#define REG_RBCPR_GCNT_TARGET(n) (0x60 + 4 * (n)) + +#define RBCPR_GCNT_TARGET_TARGET_SHIFT 0 +#define RBCPR_GCNT_TARGET_TARGET_MASK GENMASK(11, 0) +#define RBCPR_GCNT_TARGET_GCNT_SHIFT 12 +#define RBCPR_GCNT_TARGET_GCNT_MASKGENMASK(9, 0) + +/* RBCPR Timer Control */ +#define REG_RBCPR_TIMER_INTERVAL 0x44 +#define REG_RBIF_TIMER_ADJUST 0x4c + +#define RBIF_TIMER_ADJ_CONS_UP_MASKGENMASK(3, 0) +#define RBIF_TIMER_ADJ_CONS_UP_SHIFT 0 +#define RBIF_TIMER_ADJ_CONS_DOWN_MASK GENMASK(3, 0) +#define RBIF_TIMER_ADJ_CONS_DOWN_SHIFT 4 +#define RBIF_TIMER_ADJ_CLAMP_INT_MASK GENMASK(7, 0) +#de
[PATCH v2 07/14] cpufreq: qcom: Add support for qcs404 on nvmem driver
Add support for qcs404 on nvmem driver.
The qcs404 SoC has support for Core Power Reduction (CPR), which is
implemented as a power domain provider, therefore add optional support
in this driver to attach to a genpd power domain.
Co-developed-by: Jorge Ramirez-Ortiz
Signed-off-by: Jorge Ramirez-Ortiz
Signed-off-by: Niklas Cassel
---
Changes since V1:
-Adapt to dev_pm_opp_attach_genpd() API change.
drivers/cpufreq/qcom-cpufreq-nvmem.c | 50 ++--
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c
b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index 2d798a1685c5..f0d2d5035413 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -49,10 +50,12 @@ struct qcom_cpufreq_match_data {
int (*get_version)(struct device *cpu_dev,
struct nvmem_cell *speedbin_nvmem,
struct qcom_cpufreq_drv *drv);
+ const char **genpd_names;
};
struct qcom_cpufreq_drv {
struct opp_table **opp_tables;
+ struct opp_table **genpd_opp_tables;
u32 versions;
const struct qcom_cpufreq_match_data *data;
};
@@ -126,6 +129,12 @@ static const struct qcom_cpufreq_match_data
match_data_kryo = {
.get_version = qcom_cpufreq_kryo_name_version,
};
+static const char *qcs404_genpd_names[] = { "cpr", NULL };
+
+static const struct qcom_cpufreq_match_data match_data_qcs404 = {
+ .genpd_names = qcs404_genpd_names,
+};
+
static int qcom_cpufreq_probe(struct platform_device *pdev)
{
struct qcom_cpufreq_drv *drv;
@@ -188,11 +197,19 @@ static int qcom_cpufreq_probe(struct platform_device
*pdev)
goto free_drv;
}
+ drv->genpd_opp_tables = kcalloc(num_possible_cpus(),
+ sizeof(*drv->genpd_opp_tables),
+ GFP_KERNEL);
+ if (!drv->genpd_opp_tables) {
+ ret = -ENOMEM;
+ goto free_opp;
+ }
+
for_each_possible_cpu(cpu) {
cpu_dev = get_cpu_device(cpu);
if (NULL == cpu_dev) {
ret = -ENODEV;
- goto free_opp;
+ goto free_genpd_opp;
}
if (drv->data->get_version) {
@@ -203,7 +220,22 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
ret = PTR_ERR(drv->opp_tables[cpu]);
dev_err(cpu_dev,
"Failed to set supported hardware\n");
- goto free_opp;
+ goto free_genpd_opp;
+ }
+ }
+
+ if (drv->data->genpd_names) {
+ drv->genpd_opp_tables[cpu] =
+ dev_pm_opp_attach_genpd(cpu_dev,
+ drv->data->genpd_names,
+ NULL);
+ if (IS_ERR(drv->genpd_opp_tables[cpu])) {
+ ret = PTR_ERR(drv->genpd_opp_tables[cpu]);
+ if (ret != -EPROBE_DEFER)
+ dev_err(cpu_dev,
+ "Could not attach to pm_domain:
%d\n",
+ ret);
+ goto free_genpd_opp;
}
}
}
@@ -218,6 +250,13 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
ret = PTR_ERR(cpufreq_dt_pdev);
dev_err(cpu_dev, "Failed to register platform device\n");
+free_genpd_opp:
+ for_each_possible_cpu(cpu) {
+ if (IS_ERR_OR_NULL(drv->genpd_opp_tables[cpu]))
+ break;
+ dev_pm_opp_detach_genpd(drv->genpd_opp_tables[cpu]);
+ }
+ kfree(drv->genpd_opp_tables);
free_opp:
for_each_possible_cpu(cpu) {
if (IS_ERR_OR_NULL(drv->opp_tables[cpu]))
@@ -238,11 +277,15 @@ static int qcom_cpufreq_remove(struct platform_device
*pdev)
platform_device_unregister(cpufreq_dt_pdev);
- for_each_possible_cpu(cpu)
+ for_each_possible_cpu(cpu) {
if (drv->opp_tables[cpu])
dev_pm_opp_put_supported_hw(drv->opp_tables[cpu]);
+ if (drv->genpd_opp_tables[cpu])
+ dev_pm_opp_detach_genpd(drv->genpd_opp_tables[cpu]);
+ }
kfree(drv->opp_tables);
+ kfree(drv->genpd_opp_tables);
kfree(drv);
return 0;
@@ -259,6 +302,7 @@ static struct pl
[PATCH v2 06/14] dt-bindings: cpufreq: qcom-nvmem: Support pstates provided by a power domain
Some Qualcomm SoCs have support for Core Power Reduction (CPR).
On these platforms, we need to attach to the power domain provider
providing the performance states, so that the leaky device (the CPU)
can configure the performance states (which represent different
CPU clock frequencies).
Signed-off-by: Niklas Cassel
---
.../bindings/opp/qcom-nvmem-cpufreq.txt | 111 ++
1 file changed, 111 insertions(+)
diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index c5ea8b90e35d..e19a95318e98 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -23,6 +23,15 @@ In 'operating-points-v2' table:
Optional properties:
+In 'cpus' nodes:
+- power-domains: A phandle pointing to the PM domain specifier which provides
+ the performance states available for active state management.
+ Please refer to the power-domains bindings
+ Documentation/devicetree/bindings/power/power_domain.txt
+ and also examples below.
+- power-domain-names: Should be
+ - 'cpr' for qcs404.
+
In 'operating-points-v2' table:
- nvmem-cells: A phandle pointing to a nvmem-cells node representing the
efuse registers that has information about the
@@ -682,3 +691,105 @@ soc {
};
};
};
+
+Example 2:
+-
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x100>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU1: cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x101>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU2: cpu@102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x102>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU3: cpu@103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x103>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+ };
+
+ cpu_opp_table: cpu-opp-table {
+ compatible = "operating-points-v2-kryo-cpu";
+ opp-shared;
+
+ opp-109440 {
+ opp-hz = /bits/ 64 <109440>;
+ required-opps = <_opp1>;
+ };
+ opp-124800 {
+ opp-hz = /bits/ 64 <124800>;
+ required-opps = <_opp2>;
+ };
+ opp-140160 {
+ opp-hz = /bits/ 64 <140160>;
+ required-opps = <_opp3>;
+ };
+ };
+
+ cpr_opp_table: cpr-opp-table {
+ compatible = "operating-points-v2-qcom-level";
+
+ cpr_opp1: opp1 {
+ opp-level = <1>;
+
+ };
+ cpr_opp2: opp2 {
+ opp-level = <2>;
+
+ };
+ cpr_opp3: opp3 {
+ opp-level = <3>;
+
+ };
+ };
+
+
+
+soc {
+
+ cprpd: cpr@b018000 {
+ compatible = "qcom,qcs404-cpr", "qcom,cpr";
+ reg = <0x0b018000 0x1000>;
+
+ vdd-apc-supply = <_s3>;
+ #power-domain-cells = <0>;
+ operating-points-v2 = <_opp_table>;
+
+ };
+};
--
2.21.0
[PATCH v2 05/14] cpufreq: qcom: Refactor the driver to make it easier to extend
Refactor the driver to make it easier to extend in a later commit.
Create a driver struct to collect all common resources, in order to make
it easier to free up all common resources.
Create a driver match_data struct to make it easier to extend the driver
with support for new features that might only be supported on certain SoCs.
Co-developed-by: Jorge Ramirez-Ortiz
Signed-off-by: Jorge Ramirez-Ortiz
Signed-off-by: Niklas Cassel
Reviewed-by: Ilia Lin
---
Changes since V1:
-Picked up tags.
-Fixed an incorrectly placed of_node_put().
drivers/cpufreq/qcom-cpufreq-nvmem.c | 123 +--
1 file changed, 79 insertions(+), 44 deletions(-)
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c
b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index fd08120768af..2d798a1685c5 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -43,6 +43,20 @@ enum _msm8996_version {
NUM_OF_MSM8996_VERSIONS,
};
+struct qcom_cpufreq_drv;
+
+struct qcom_cpufreq_match_data {
+ int (*get_version)(struct device *cpu_dev,
+ struct nvmem_cell *speedbin_nvmem,
+ struct qcom_cpufreq_drv *drv);
+};
+
+struct qcom_cpufreq_drv {
+ struct opp_table **opp_tables;
+ u32 versions;
+ const struct qcom_cpufreq_match_data *data;
+};
+
static struct platform_device *cpufreq_dt_pdev, *cpufreq_pdev;
static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
@@ -76,7 +90,7 @@ static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
struct nvmem_cell *speedbin_nvmem,
- u32 *versions)
+ struct qcom_cpufreq_drv *drv)
{
size_t len;
u8 *speedbin;
@@ -94,10 +108,10 @@ static int qcom_cpufreq_kryo_name_version(struct device
*cpu_dev,
switch (msm8996_version) {
case MSM8996_V3:
- *versions = 1 << (unsigned int)(*speedbin);
+ drv->versions = 1 << (unsigned int)(*speedbin);
break;
case MSM8996_SG:
- *versions = 1 << ((unsigned int)(*speedbin) + 4);
+ drv->versions = 1 << ((unsigned int)(*speedbin) + 4);
break;
default:
BUG();
@@ -108,17 +122,17 @@ static int qcom_cpufreq_kryo_name_version(struct device
*cpu_dev,
return 0;
}
+static const struct qcom_cpufreq_match_data match_data_kryo = {
+ .get_version = qcom_cpufreq_kryo_name_version,
+};
+
static int qcom_cpufreq_probe(struct platform_device *pdev)
{
- struct opp_table **opp_tables;
- int (*get_version)(struct device *cpu_dev,
- struct nvmem_cell *speedbin_nvmem,
- u32 *versions);
+ struct qcom_cpufreq_drv *drv;
struct nvmem_cell *speedbin_nvmem;
struct device_node *np;
struct device *cpu_dev;
unsigned cpu;
- u32 versions;
const struct of_device_id *match;
int ret;
@@ -126,11 +140,6 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
if (!cpu_dev)
return -ENODEV;
- match = pdev->dev.platform_data;
- get_version = match->data;
- if (!get_version)
- return -ENODEV;
-
np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
if (!np)
return -ENOENT;
@@ -141,23 +150,43 @@ static int qcom_cpufreq_probe(struct platform_device
*pdev)
return -ENOENT;
}
- speedbin_nvmem = of_nvmem_cell_get(np, NULL);
- of_node_put(np);
- if (IS_ERR(speedbin_nvmem)) {
- if (PTR_ERR(speedbin_nvmem) != -EPROBE_DEFER)
- dev_err(cpu_dev, "Could not get nvmem cell: %ld\n",
- PTR_ERR(speedbin_nvmem));
- return PTR_ERR(speedbin_nvmem);
+ drv = kzalloc(sizeof(*drv), GFP_KERNEL);
+ if (!drv)
+ return -ENOMEM;
+
+ match = pdev->dev.platform_data;
+ drv->data = match->data;
+ if (!drv->data) {
+ ret = -ENODEV;
+ goto free_drv;
}
- ret = get_version(cpu_dev, speedbin_nvmem, );
- nvmem_cell_put(speedbin_nvmem);
- if (ret)
- return ret;
+ if (drv->data->get_version) {
+ speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+ if (IS_ERR(speedbin_nvmem)) {
+ if (PTR_ERR(speedbin_nvmem) != -EPROBE_DEFER)
+ dev_err(cpu_dev,
+ "Could not get nvmem cell: %ld\n",
+ PTR_ERR(speedbin_nvmem));
+ ret = PTR_ERR(speedbin_nvmem);
+ goto free_drv;
+
[PATCH v2 04/14] dt-bindings: cpufreq: qcom-nvmem: Make speedbin related properties optional
Not all Qualcomm platforms need to care about the speedbin efuse, nor the value blown into the speedbin efuse. Therefore, make the nvmem-cells and opp-supported-hw properties optional. Signed-off-by: Niklas Cassel Reviewed-by: Ilia Lin Reviewed-by: Rob Herring --- Changes since V1: -Picked up tags. Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt index 198441e80ba8..c5ea8b90e35d 100644 --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt @@ -20,6 +20,10 @@ In 'cpus' nodes: In 'operating-points-v2' table: - compatible: Should be - 'operating-points-v2-kryo-cpu' for apq8096 and msm8996. + +Optional properties: + +In 'operating-points-v2' table: - nvmem-cells: A phandle pointing to a nvmem-cells node representing the efuse registers that has information about the speedbin that is used to select the right frequency/voltage -- 2.21.0
[PATCH v2 03/14] cpufreq: qcom: Re-organise kryo cpufreq to use it for other nvmem based qcom socs
From: Sricharan R
The kryo cpufreq driver reads the nvmem cell and uses that data to
populate the opps. There are other qcom cpufreq socs like krait which
does similar thing. Except for the interpretation of the read data,
rest of the driver is same for both the cases. So pull the common things
out for reuse.
Signed-off-by: Sricharan R
[niklas.cas...@linaro.org: split dt-binding into a separate patch and
do not rename the compatible string. Update MAINTAINERS file.]
Signed-off-by: Niklas Cassel
Reviewed-by: Ilia Lin
---
Changes since V1:
-Picked up tags.
-Renamed .driver .name to "qcom-cpufreq-nvmem".
MAINTAINERS | 4 +-
drivers/cpufreq/Kconfig.arm | 4 +-
drivers/cpufreq/Makefile | 2 +-
...om-cpufreq-kryo.c => qcom-cpufreq-nvmem.c} | 122 +++---
4 files changed, 78 insertions(+), 54 deletions(-)
rename drivers/cpufreq/{qcom-cpufreq-kryo.c => qcom-cpufreq-nvmem.c} (69%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 711b5d07f73d..d6b42e2413e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13320,8 +13320,8 @@ QUALCOMM CPUFREQ DRIVER MSM8996/APQ8096
M: Ilia Lin
L: linux...@vger.kernel.org
S: Maintained
-F: Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
-F: drivers/cpufreq/qcom-cpufreq-kryo.c
+F: Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
+F: drivers/cpufreq/qcom-cpufreq-nvmem.c
QUALCOMM EMAC GIGABIT ETHERNET DRIVER
M: Timur Tabi
diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 70c2b4bea55c..a905796f7f85 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -132,8 +132,8 @@ config ARM_OMAP2PLUS_CPUFREQ
depends on ARCH_OMAP2PLUS
default ARCH_OMAP2PLUS
-config ARM_QCOM_CPUFREQ_KRYO
- tristate "Qualcomm Kryo based CPUFreq"
+config ARM_QCOM_CPUFREQ_NVMEM
+ tristate "Qualcomm nvmem based CPUFreq"
depends on ARM64
depends on QCOM_QFPROM
depends on QCOM_SMEM
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 7f2d2e1079d4..9a9f5ccd13d9 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -64,7 +64,7 @@ obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o
obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o
obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o
obj-$(CONFIG_ARM_QCOM_CPUFREQ_HW) += qcom-cpufreq-hw.o
-obj-$(CONFIG_ARM_QCOM_CPUFREQ_KRYO)+= qcom-cpufreq-kryo.o
+obj-$(CONFIG_ARM_QCOM_CPUFREQ_NVMEM) += qcom-cpufreq-nvmem.o
obj-$(CONFIG_ARM_RASPBERRYPI_CPUFREQ) += raspberrypi-cpufreq.o
obj-$(CONFIG_ARM_S3C2410_CPUFREQ) += s3c2410-cpufreq.o
obj-$(CONFIG_ARM_S3C2412_CPUFREQ) += s3c2412-cpufreq.o
diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c
b/drivers/cpufreq/qcom-cpufreq-nvmem.c
similarity index 69%
rename from drivers/cpufreq/qcom-cpufreq-kryo.c
rename to drivers/cpufreq/qcom-cpufreq-nvmem.c
index dd64dcf89c74..fd08120768af 100644
--- a/drivers/cpufreq/qcom-cpufreq-kryo.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -9,7 +9,7 @@
* based on the silicon variant in use. Qualcomm Process Voltage Scaling Tables
* defines the voltage and frequency value based on the msm-id in SMEM
* and speedbin blown in the efuse combination.
- * The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
+ * The qcom-cpufreq-nvmem driver reads the msm-id and efuse value from the SoC
* to provide the OPP framework with required information.
* This is used to determine the voltage and frequency value for each OPP of
* operating-points-v2 table when it is parsed by the OPP framework.
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -42,9 +43,9 @@ enum _msm8996_version {
NUM_OF_MSM8996_VERSIONS,
};
-static struct platform_device *cpufreq_dt_pdev, *kryo_cpufreq_pdev;
+static struct platform_device *cpufreq_dt_pdev, *cpufreq_pdev;
-static enum _msm8996_version qcom_cpufreq_kryo_get_msm_id(void)
+static enum _msm8996_version qcom_cpufreq_get_msm_id(void)
{
size_t len;
u32 *msm_id;
@@ -73,28 +74,62 @@ static enum _msm8996_version
qcom_cpufreq_kryo_get_msm_id(void)
return version;
}
-static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
+static int qcom_cpufreq_kryo_name_version(struct device *cpu_dev,
+ struct nvmem_cell *speedbin_nvmem,
+ u32 *versions)
{
- struct opp_table **opp_tables;
+ size_t len;
+ u8 *speedbin;
enum _msm8996_version msm8996_version;
+
+ msm8996_version = qcom_cpufreq_get_msm_id();
+ if (NUM_OF_MSM8996_VERSIONS == msm8996_version) {
+ dev_err(cpu_dev, "Not Snapdragon 820/821!");
+ return -ENODEV;
+ }
+
+ speedbin = nvmem_cell_read(
[PATCH v2 02/14] dt-bindings: cpufreq: Re-organise kryo cpufreq to use it for other nvmem based qcom socs
From: Sricharan R
The kryo cpufreq driver reads the nvmem cell and uses that data to
populate the opps. There are other qcom cpufreq socs like krait which
does similar thing. Except for the interpretation of the read data,
rest of the driver is same for both the cases. So pull the common things
out for reuse.
Signed-off-by: Sricharan R
[niklas.cas...@linaro.org: split dt-binding into a separate patch and
do not rename the compatible string.]
Signed-off-by: Niklas Cassel
Reviewed-by: Ilia Lin
Reviewed-by: Rob Herring
---
Changes since V1:
-Picked up tags.
.../opp/{kryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
rename Documentation/devicetree/bindings/opp/{kryo-cpufreq.txt =>
qcom-nvmem-cpufreq.txt} (98%)
diff --git a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
similarity index 98%
rename from Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
rename to Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index c2127b96805a..198441e80ba8 100644
--- a/Documentation/devicetree/bindings/opp/kryo-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -1,13 +1,13 @@
-Qualcomm Technologies, Inc. KRYO CPUFreq and OPP bindings
+Qualcomm Technologies, Inc. NVMEM CPUFreq and OPP bindings
===
-In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996
-that have KRYO processors, the CPU ferequencies subset and voltage value
-of each OPP varies based on the silicon variant in use.
+In Certain Qualcomm Technologies, Inc. SoCs like apq8096 and msm8996,
+the CPU frequencies subset and voltage value of each OPP varies based on
+the silicon variant in use.
Qualcomm Technologies, Inc. Process Voltage Scaling Tables
defines the voltage and frequency value based on the msm-id in SMEM
and speedbin blown in the efuse combination.
-The qcom-cpufreq-kryo driver reads the msm-id and efuse value from the SoC
+The qcom-cpufreq-nvmem driver reads the msm-id and efuse value from the SoC
to provide the OPP framework with required information (existing HW bitmap).
This is used to determine the voltage and frequency value for each OPP of
operating-points-v2 table when it is parsed by the OPP framework.
--
2.21.0
[PATCH v2 00/14] Add support for QCOM Core Power Reduction
This series adds support for Core Power Reduction (CPR), a form of
Adaptive Voltage Scaling (AVS), found on certain Qualcomm SoCs.
This series is based on top of the qcs404 cpufreq patch series that
hasn't landed yet:
https://patchwork.kernel.org/project/linux-arm-msm/list/?series=137809
CPR is a technology that reduces core power on a CPU or on other device.
It reads voltage settings from efuses (that have been written in
production), it uses these voltage settings as initial values, for each
OPP.
After moving to a certain OPP, CPR monitors dynamic factors such as
temperature, etc. and adjusts the voltage for that frequency accordingly
to save power and meet silicon characteristic requirements.
This driver has been developed together with Jorge Ramirez-Ortiz, and
is based on an RFC by Stephen Boyd[1], which in turn is based on work
by others on codeaurora.org[2].
[1] https://lkml.org/lkml/2015/9/18/833
[2]
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.10/tree/drivers/regulator/cpr-regulator.c?h=msm-3.10
Changes since V1:
Added a new patch implementing dev_pm_opp_find_level_exact() in order to
make the CPR OPP table in device tree cleaner.
For more detailed changes, check the "Changes since V1" as comments in
the individual patches, where applicable.
Jorge Ramirez-Ortiz (1):
cpufreq: Add qcs404 to cpufreq-dt-platdev blacklist
Niklas Cassel (11):
opp: Add dev_pm_opp_find_level_exact()
dt-bindings: cpufreq: qcom-nvmem: Make speedbin related properties
optional
cpufreq: qcom: Refactor the driver to make it easier to extend
dt-bindings: cpufreq: qcom-nvmem: Support pstates provided by a power
domain
cpufreq: qcom: Add support for qcs404 on nvmem driver
dt-bindings: opp: Add qcom-opp bindings with properties needed for CPR
dt-bindings: power: avs: Add support for CPR (Core Power Reduction)
power: avs: Add support for CPR (Core Power Reduction)
arm64: dts: qcom: qcs404: Add CPR and populate OPP table
arm64: defconfig: enable CONFIG_QCOM_CPR
arm64: defconfig: enable CONFIG_ARM_QCOM_CPUFREQ_NVMEM
Sricharan R (2):
dt-bindings: cpufreq: Re-organise kryo cpufreq to use it for other
nvmem based qcom socs
cpufreq: qcom: Re-organise kryo cpufreq to use it for other nvmem
based qcom socs
...ryo-cpufreq.txt => qcom-nvmem-cpufreq.txt} | 125 +-
.../devicetree/bindings/opp/qcom-opp.txt | 19 +
.../bindings/power/avs/qcom,cpr.txt | 193 ++
MAINTAINERS | 13 +-
arch/arm64/boot/dts/qcom/qcs404.dtsi | 142 +-
arch/arm64/configs/defconfig |2 +
drivers/cpufreq/Kconfig.arm |4 +-
drivers/cpufreq/Makefile |2 +-
drivers/cpufreq/cpufreq-dt-platdev.c |1 +
drivers/cpufreq/qcom-cpufreq-kryo.c | 249 ---
drivers/cpufreq/qcom-cpufreq-nvmem.c | 352 +++
drivers/opp/core.c| 48 +
drivers/power/avs/Kconfig | 15 +
drivers/power/avs/Makefile|1 +
drivers/power/avs/qcom-cpr.c | 1885 +
include/linux/pm_opp.h|8 +
16 files changed, 2792 insertions(+), 267 deletions(-)
rename Documentation/devicetree/bindings/opp/{kryo-cpufreq.txt =>
qcom-nvmem-cpufreq.txt} (87%)
create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt
create mode 100644 Documentation/devicetree/bindings/power/avs/qcom,cpr.txt
delete mode 100644 drivers/cpufreq/qcom-cpufreq-kryo.c
create mode 100644 drivers/cpufreq/qcom-cpufreq-nvmem.c
create mode 100644 drivers/power/avs/qcom-cpr.c
--
2.21.0
[PATCH v2 01/14] opp: Add dev_pm_opp_find_level_exact()
When using performance states, there is usually not any opp-hz property
specified, so the dev_pm_opp_find_freq_exact() function cannot be used.
Since the performance states in the OPP table are unique, implement a
dev_pm_opp_find_level_exact() in order to be able to fetch a specific OPP.
Signed-off-by: Niklas Cassel
---
drivers/opp/core.c | 48 ++
include/linux/pm_opp.h | 8 +++
2 files changed, 56 insertions(+)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index cac3e4005045..3b7ffd0234e9 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -401,6 +401,54 @@ struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct
device *dev,
}
EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_exact);
+/**
+ * dev_pm_opp_find_level_exact() - search for an exact level
+ * @dev: device for which we do this operation
+ * @level: level to search for
+ *
+ * Return: Searches for exact match in the opp table and returns pointer to the
+ * matching opp if found, else returns ERR_PTR in case of error and should
+ * be handled using IS_ERR. Error return values can be:
+ * EINVAL: for bad pointer
+ * ERANGE: no match found for search
+ * ENODEV: if device not found in list of registered devices
+ *
+ * The callers are required to call dev_pm_opp_put() for the returned OPP after
+ * use.
+ */
+struct dev_pm_opp *dev_pm_opp_find_level_exact(struct device *dev,
+ unsigned int level)
+{
+ struct opp_table *opp_table;
+ struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE);
+
+ opp_table = _find_opp_table(dev);
+ if (IS_ERR(opp_table)) {
+ int r = PTR_ERR(opp_table);
+
+ dev_err(dev, "%s: OPP table not found (%d)\n", __func__, r);
+ return ERR_PTR(r);
+ }
+
+ mutex_lock(_table->lock);
+
+ list_for_each_entry(temp_opp, _table->opp_list, node) {
+ if (temp_opp->level == level) {
+ opp = temp_opp;
+
+ /* Increment the reference count of OPP */
+ dev_pm_opp_get(opp);
+ break;
+ }
+ }
+
+ mutex_unlock(_table->lock);
+ dev_pm_opp_put_opp_table(opp_table);
+
+ return opp;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_find_level_exact);
+
static noinline struct dev_pm_opp *_find_freq_ceil(struct opp_table *opp_table,
unsigned long *freq)
{
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 5bdceca5125d..b8197ab014f2 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -96,6 +96,8 @@ unsigned long dev_pm_opp_get_suspend_opp_freq(struct device
*dev);
struct dev_pm_opp *dev_pm_opp_find_freq_exact(struct device *dev,
unsigned long freq,
bool available);
+struct dev_pm_opp *dev_pm_opp_find_level_exact(struct device *dev,
+ unsigned int level);
struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev,
unsigned long *freq);
@@ -200,6 +202,12 @@ static inline struct dev_pm_opp
*dev_pm_opp_find_freq_exact(struct device *dev,
return ERR_PTR(-ENOTSUPP);
}
+static inline struct dev_pm_opp *dev_pm_opp_find_level_exact(struct device
*dev,
+ unsigned int level)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+
static inline struct dev_pm_opp *dev_pm_opp_find_freq_floor(struct device *dev,
unsigned long *freq)
{
--
2.21.0
Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table
On Tue, Jul 23, 2019 at 07:26:35AM +0530, Viresh Kumar wrote: > On 19-07-19, 17:45, Niklas Cassel wrote: > > Hello Viresh, > > > > Could you please have a look at the last two patches here: > > https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-opp-hz > > There is no sane way of providing review comments with a link to the > git tree :) > > I still had a look and I see that you don't search for max frequency > but just any OPP that has required-opps set to the level u want. Also, > can't there be multiple phandles in required-opps in your case ? For each OPP in the CPR OPP table, we need three things, opp-level, qcom,fuse-level and opp-hz. The first two can simply be parsed from the OPP node itself while iterating the CPR OPP table. The opp-hz has to be fetched from the CPU OPP table. Several OPPs might have the same qcom,fuse-level value. However, they will have unique opp-level values and unique opp-hz values. Each opp-level has a matching opp-hz. required-opps is basically a connection between a opp-hz (CPU OPP table) and and a opp-level (CPR OPP table). So there will be only one match. No need to search for max frequency. I think you are confusing this with something else. The CPR hardware has to be programmed with the highest frequency for each qcom,fuse-corner. This is done here: https://git.linaro.org/people/niklas.cassel/kernel.git/tree/drivers/power/avs/qcom-cpr.c?h=cpr-full#n1219 by saving the highest frequency for each fuse level while iterating the OPP table. There can be only one phandle in the required-opps in my case, this is one of the reasons why I prefer implementing it in the CPR driver. If it were to be implemented in OPP core, it probably has to handle multiple phandles. > > > If you like my proposal then I could send out the first patch (the one to > > OPP core) as a real patch (with an improved commit message), and > > incorporate the second patch into my CPR patch series when I send out a V2. > > Send them both in your series only, otherwise the first one is useless > anyway. Ok, will do. Kind regards, Niklas
Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table
On Wed, Jul 17, 2019 at 10:19:23AM +0530, Viresh Kumar wrote:
> On 16-07-19, 12:53, Niklas Cassel wrote:
> > Here I cheated and simply used get_cpu_device(0).
> >
> > Since I cheated, I used get_cpu_device(0) always,
> > so even when CPU1,CPU2,CPU3 is attached, dev_pm_opp_get_opp_count(cpu0) is
> > still 0.
> >
> > I added a print in
> > [3.836533] cpr_set_performance: number of OPPs for dev: cpu0: 3
> >
> > And there I can see that OPP count is 3, so it appears that with the
> > current code, we need to wait until cpufreq-dt.c:cpufreq_init()
> > has been called, maybe dev_pm_opp_of_cpumask_add_table() needs
> > to be called before dev_pm_opp_get_opp_count(cpu0) actually returns 3.
> >
> > cpufreq_init() is called by platform_device_register_simple("cpufreq-dt",
> > -1,
> > NULL, 0);
> > which is called after dev_pm_opp_attach_genpd().
> >
> > What I don't understand is that dev_pm_opp_attach_genpd() actually returns
> > a OPP table. So why do we need to wait for
> > dev_pm_opp_of_cpumask_add_table(),
> > before either dev_pm_opp_get_opp_count(cpu0) or
> > dev_pm_opp_get_opp_count(genpd_virtdev_for_cpu0) returns 3?
>
> Ah, I see the problems now. No, cpufreq table can't be available at
> this point of time and we aren't going to change that. It is the right
> thing to do.
>
> Now, even if the kernel isn't written in a way which works for you, it
> isn't right to put more things in DT than required. DT is (should be)
> very much independent of the Linux kernel.
>
> So we have to parse DT to find highest frequency for each
> required-opp. Best is to put that code in the OPP core and use it from
> your driver.
Hello Viresh,
Could you please have a look at the last two patches here:
https://git.linaro.org/people/niklas.cassel/kernel.git/log/?h=cpr-opp-hz
If you like my proposal then I could send out the first patch (the one to
OPP core) as a real patch (with an improved commit message), and
incorporate the second patch into my CPR patch series when I send out a V2.
Kind regards,
Niklas
Re: [PATCH 02/13] cpufreq: qcom: Re-organise kryo cpufreq to use it for other nvmem based qcom socs
On Wed, Jul 10, 2019 at 11:48:39AM +0530, Viresh Kumar wrote:
> On 05-07-19, 11:57, Niklas Cassel wrote:
> > -static struct platform_driver qcom_cpufreq_kryo_driver = {
> > - .probe = qcom_cpufreq_kryo_probe,
> > - .remove = qcom_cpufreq_kryo_remove,
> > +static struct platform_driver qcom_cpufreq_driver = {
> > + .probe = qcom_cpufreq_probe,
> > + .remove = qcom_cpufreq_remove,
> > .driver = {
> > - .name = "qcom-cpufreq-kryo",
> > + .name = "qcom-cpufreq",
>
> Should we still name it "qcom-cpufreq-nvmem" here ? Only the string
> here.
Sure, I can fix this in next version.
Kind regards,
Niklas
>
> > },
> > };
>
> --
> viresh
Re: [PATCH 04/13] cpufreq: qcom: Refactor the driver to make it easier to extend
On Wed, Jul 10, 2019 at 12:00:26PM +0530, Viresh Kumar wrote:
> On 05-07-19, 11:57, Niklas Cassel wrote:
> > + drv->opp_tables = kcalloc(num_possible_cpus(), sizeof(*drv->opp_tables),
> > + GFP_KERNEL);
> > + if (!drv->opp_tables) {
> > + ret = -ENOMEM;
> > + goto free_drv;
> > + }
> >
> > for_each_possible_cpu(cpu) {
> > cpu_dev = get_cpu_device(cpu);
> > @@ -166,19 +195,23 @@ static int qcom_cpufreq_probe(struct platform_device
> > *pdev)
> > goto free_opp;
> > }
> >
> > - opp_tables[cpu] = dev_pm_opp_set_supported_hw(cpu_dev,
> > - , 1);
> > - if (IS_ERR(opp_tables[cpu])) {
> > - ret = PTR_ERR(opp_tables[cpu]);
> > - dev_err(cpu_dev, "Failed to set supported hardware\n");
> > - goto free_opp;
> > + if (drv->data->get_version) {
>
> Why depend on get_version here ? The OPP table is already allocated
> unconditionally.
Since the reading of the speedbin efuse is now optional,
it is now inside "if (drv->data->get_version)".
So if I don't also protect "dev_pm_opp_set_supported_hw()"
with "if (drv->data->get_version)" I get:
[3.135092] cpu cpu0: _opp_is_supported: failed to read opp-supported-hw
property at index 0: -22
[3.139364] cpu cpu0: _opp_is_supported: failed to read opp-supported-hw
property at index 0: -22
[3.148330] cpu cpu0: _opp_is_supported: failed to read opp-supported-hw
property at index 0: -22
Probably since drv->versions is initialized to 0,
and if there is no opp-supported-hw in device tree,
OPP framework prints failures.
So it feels safest to only call dev_pm_opp_set_supported_hw()
if we know that we are supposed to parse the speedbin efuse.
Kind regards,
Niklas
>
> > + drv->opp_tables[cpu] =
> > + dev_pm_opp_set_supported_hw(cpu_dev,
> > + >versions, 1);
> > + if (IS_ERR(drv->opp_tables[cpu])) {
> > + ret = PTR_ERR(drv->opp_tables[cpu]);
> > + dev_err(cpu_dev,
> > + "Failed to set supported hardware\n");
> > + goto free_opp;
> > + }
> > }
> > }
>
> --
> viresh
Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table
On Tue, Jul 16, 2019 at 04:04:36PM +0530, Viresh Kumar wrote:
> On 15-07-19, 15:24, Niklas Cassel wrote:
> > This was actually my initial thought when talking to you 6+ months ago.
> > However, the problem was that, from the CPR drivers' perspective, it
> > only sees the CPR OPP table.
> >
> >
> > So this is the order things are called,
> > from qcom-cpufreq-nvmem.c perspective:
> >
> > 1) dev_pm_opp_set_supported_hw()
> >
> > 2) dev_pm_opp_attach_genpd() ->
> > which results in
> > int cpr_pd_attach_dev(struct generic_pm_domain *domain,
> > struct device *dev)
> > being called.
> > This callback is inside the CPR driver, and here we have the
> > CPU's (genpd virtual) struct device, and this is where we would like to
> > know the opp-hz.
> > The problem here is that:
> > [3.114979] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev:
> > genpd:0:cpu0: -19
> > [3.119610] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: cpu0: 0
Here I cheated and simply used get_cpu_device(0).
Since I cheated, I used get_cpu_device(0) always,
so even when CPU1,CPU2,CPU3 is attached, dev_pm_opp_get_opp_count(cpu0) is
still 0.
I added a print in
[3.836533] cpr_set_performance: number of OPPs for dev: cpu0: 3
And there I can see that OPP count is 3, so it appears that with the
current code, we need to wait until cpufreq-dt.c:cpufreq_init()
has been called, maybe dev_pm_opp_of_cpumask_add_table() needs
to be called before dev_pm_opp_get_opp_count(cpu0) actually returns 3.
cpufreq_init() is called by platform_device_register_simple("cpufreq-dt", -1,
NULL, 0);
which is called after dev_pm_opp_attach_genpd().
What I don't understand is that dev_pm_opp_attach_genpd() actually returns
a OPP table. So why do we need to wait for dev_pm_opp_of_cpumask_add_table(),
before either dev_pm_opp_get_opp_count(cpu0) or
dev_pm_opp_get_opp_count(genpd_virtdev_for_cpu0) returns 3?
> > [3.126489] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev:
> > cpr@b018000: 3
> >
> > While we have the CPR OPP table in the attach callback, we don't
> > have the CPU OPP table, neither in the CPU struct device or the genpd
> > virtual
> > struct device.
>
> If you can find CPU's physical number from the virtual device, then
> you can do get_cpu_device(X) and then life will be easy ?
>
> > Since we have called dev_pm_opp_attach_genpd(.., .., _devs) which
> > attaches an OPP table to the CPU, I would have expected one of them to
> > be >= 0.
> > Especially since dev_name(virt_devs[0]) == genpd:0:cpu0
> >
> > I guess it should still be possible to parse the required-opps manually
> > here,
> > by iterating the OF nodes, however, we won't be able to use the CPU's struct
> > opp_table (which is the nice representation of the OF nodes).
> >
> > Any suggestions?
>
> --
> viresh
Re: [PATCH] opp: Return genpd virtual devices from dev_pm_opp_attach_genpd()
On Mon, Jul 08, 2019 at 11:30:11AM +0530, Viresh Kumar wrote:
> The cpufreq drivers don't need to do runtime PM operations on the
> virtual devices returned by dev_pm_domain_attach_by_name() and so the
> virtual devices weren't shared with the callers of
> dev_pm_opp_attach_genpd() earlier.
>
> But the IO device drivers would want to do that. This patch updates the
> prototype of dev_pm_opp_attach_genpd() to accept another argument to
> return the pointer to the array of genpd virtual devices.
>
> Reported-by: Rajendra Nayak
> Signed-off-by: Viresh Kumar
> ---
> @Rajendra: Can you please test this one ? I have only compile tested it.
>
> drivers/opp/core.c | 5 -
> include/linux/pm_opp.h | 4 ++--
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 2958cc7bbb58..07b6f1187b3b 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1775,6 +1775,7 @@ static void _opp_detach_genpd(struct opp_table
> *opp_table)
> * dev_pm_opp_attach_genpd - Attach genpd(s) for the device and save virtual
> device pointer
> * @dev: Consumer device for which the genpd is getting attached.
> * @names: Null terminated array of pointers containing names of genpd to
> attach.
> + * @virt_devs: Pointer to return the array of virtual devices.
> *
> * Multiple generic power domains for a device are supported with the help of
> * virtual genpd devices, which are created for each consumer device - genpd
> @@ -1789,7 +1790,8 @@ static void _opp_detach_genpd(struct opp_table
> *opp_table)
> * This helper needs to be called once with a list of all genpd to attach.
> * Otherwise the original device structure will be used instead by the OPP
> core.
> */
> -struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char
> **names)
> +struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
> + const char **names, struct device ***virt_devs)
> {
> struct opp_table *opp_table;
> struct device *virt_dev;
> @@ -1850,6 +1852,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device
> *dev, const char **names
> name++;
> }
>
> + *virt_devs = opp_table->genpd_virt_devs;
Could we perhaps only do this if (virt_devs), that way callers can send in
NULL if they don't care about the genpd virtual devices.
Kind regards,
Niklas
> mutex_unlock(_table->genpd_virt_dev_lock);
>
> return opp_table;
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index be570761b77a..7c2fe2952f40 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -131,7 +131,7 @@ struct opp_table *dev_pm_opp_set_clkname(struct device
> *dev, const char * name);
> void dev_pm_opp_put_clkname(struct opp_table *opp_table);
> struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, int
> (*set_opp)(struct dev_pm_set_opp_data *data));
> void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table);
> -struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char
> **names);
> +struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char
> **names, struct device ***virt_devs);
> void dev_pm_opp_detach_genpd(struct opp_table *opp_table);
> int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct
> opp_table *dst_table, unsigned int pstate);
> int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
> @@ -295,7 +295,7 @@ static inline struct opp_table
> *dev_pm_opp_set_clkname(struct device *dev, const
>
> static inline void dev_pm_opp_put_clkname(struct opp_table *opp_table) {}
>
> -static inline struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
> const char **names)
> +static inline struct opp_table *dev_pm_opp_attach_genpd(struct device *dev,
> const char **names, struct device ***virt_devs)
> {
> return ERR_PTR(-ENOTSUPP);
> }
> --
> 2.21.0.rc0.269.g1a574e7a288b
>
Re: [PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table
On Wed, Jul 10, 2019 at 02:33:03PM +0530, Viresh Kumar wrote:
> On 05-07-19, 11:57, Niklas Cassel wrote:
> > diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > cpu_opp_table: cpu-opp-table {
> > - compatible = "operating-points-v2";
> > + compatible = "operating-points-v2-kryo-cpu";
> > opp-shared;
> >
> > opp-109440 {
> > opp-hz = /bits/ 64 <109440>;
> > - opp-microvolt = <1224000 1224000 1224000>;
> > + required-opps = <_opp1>;
> > };
> > opp-124800 {
> > opp-hz = /bits/ 64 <124800>;
> > - opp-microvolt = <1288000 1288000 1288000>;
> > + required-opps = <_opp2>;
> > };
> > opp-140160 {
> > opp-hz = /bits/ 64 <140160>;
> > - opp-microvolt = <1384000 1384000 1384000>;
> > + required-opps = <_opp3>;
> > + };
> > + };
> > +
> > + cpr_opp_table: cpr-opp-table {
> > + compatible = "operating-points-v2-qcom-level";
> > +
> > + cpr_opp1: opp1 {
> > + opp-level = <1>;
> > + qcom,opp-fuse-level = <1>;
> > + opp-hz = /bits/ 64 <109440>;
> > + };
> > + cpr_opp2: opp2 {
> > + opp-level = <2>;
> > + qcom,opp-fuse-level = <2>;
> > + opp-hz = /bits/ 64 <124800>;
> > + };
> > + cpr_opp3: opp3 {
> > + opp-level = <3>;
> > + qcom,opp-fuse-level = <3>;
> > + opp-hz = /bits/ 64 <140160>;
> > };
> > };
>
> - Do we ever have cases more complex than this for this version of CPR ?
For e.g. CPR on msm8916, we will have 7 different frequencies in the CPU
OPP table, but only 3 OPPs in the CPR OPP table.
Each of the 7 OPPs in the CPU OPP table will have a required-opps that
points to an OPP in the CPR OPP table.
On certain msm8916:s, the speedbin efuse will limit us to only have 6
OPPs in the CPU OPP table, but the required-opps are still the same.
So I would say that it is just slightly more complex..
>
> - What about multiple devices with same CPR table, not big LITTLE
> CPUs, but other devices like two different type of IO devices ? What
> will we do with opp-hz in those cases ?
On all SoCs where there is a CPR for e.g. GPU, there is an additional
CPR hardware block, so then there will also be an additional CPR OPP
table. So I don't think that this will be a problem.
>
> - If there are no such cases, can we live without opp-hz being used
> here and reverse-engineer the highest frequency by looking directly
> at CPUs OPP table ? i.e. by looking at required-opps field.
This was actually my initial thought when talking to you 6+ months ago.
However, the problem was that, from the CPR drivers' perspective, it
only sees the CPR OPP table.
So this is the order things are called,
from qcom-cpufreq-nvmem.c perspective:
1) dev_pm_opp_set_supported_hw()
2) dev_pm_opp_attach_genpd() ->
which results in
int cpr_pd_attach_dev(struct generic_pm_domain *domain,
struct device *dev)
being called.
This callback is inside the CPR driver, and here we have the
CPU's (genpd virtual) struct device, and this is where we would like to
know the opp-hz.
The problem here is that:
[3.114979] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev:
genpd:0:cpu0: -19
[3.119610] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev: cpu0: 0
[3.126489] cpr_pd_attach_dev: dev_pm_opp_get_opp_count for dev:
cpr@b018000: 3
While we have the CPR OPP table in the attach callback, we don't
have the CPU OPP table, neither in the CPU struct device or the genpd virtual
struct device.
Since we have called dev_pm_opp_attach_genpd(.., .., _devs) which
attaches an OPP table to the CPU, I would have expected one of them to
be >= 0.
Especially since dev_name(virt_devs[0]) == genpd:0:cpu0
I guess it should still be possible to parse the required-opps manually here,
by iterating the OF nodes, however, we won't be able to use the CPU's struct
opp_table (which is the nice representation of the OF nodes).
Any suggestions?
Kind regards,
Niklas
Re: [RFC PATCH 6/9] dt-bindings: opp: Add qcom-opp bindings with properties needed for CPR
On Tue, Apr 09, 2019 at 02:53:52PM +0530, Viresh Kumar wrote: > On 04-04-19, 07:09, Niklas Cassel wrote: > > Add qcom-opp bindings with properties needed for Core Power Reduction (CPR). > > > > CPR is included in a great variety of Qualcomm SoC, e.g. msm8916 and > > msm8996, > > and was first introduced in msm8974. > > > > Co-developed-by: Jorge Ramirez-Ortiz > > Signed-off-by: Jorge Ramirez-Ortiz > > Signed-off-by: Niklas Cassel > > --- > > .../devicetree/bindings/opp/qcom-opp.txt | 24 +++ > > 1 file changed, 24 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/opp/qcom-opp.txt > > > > diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt > > b/Documentation/devicetree/bindings/opp/qcom-opp.txt > > new file mode 100644 > > index ..d24280467db7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt > > @@ -0,0 +1,24 @@ > > +Qualcomm OPP bindings to describe OPP nodes > > + > > +The bindings are based on top of the operating-points-v2 bindings > > +described in Documentation/devicetree/bindings/opp/opp.txt > > +Additional properties are described below. > > + > > +* OPP Table Node > > + > > +Required properties: > > +- compatible: Allow OPPs to express their compatibility. It should be: > > + "operating-points-v2-qcom-level" > > + > > +* OPP Node > > + > > +Optional properties: > > +- opp-hz: Frequency in Hz, expressed as a 64-bit big-endian integer. Even > > + though a power domain doesn't need a opp-hz, there can be devices in the > > + power domain that need to know the highest supported frequency for each > > + corner/level (e.g. CPR), in order to properly initialize the hardware. > > + > > +- qcom,opp-fuse-level: A positive value representing the fuse corner/level > > + associated with this OPP node. Sometimes several corners/levels shares > > + a certain fuse corner/level. A fuse corner/level contains e.g. ref uV, > > + min uV, and max uV. > > I know we discussed this sometime back and so you implemented it this way. > > Looking at the implementation of the CPR driver, I now wonder if that was a > good > choice. Technically a single domain can manage many devices, a big and a > little > CPU for example and then we will have different highest frequencies for both > of > them. How will we configure the CPR hardware in such a case ? Isn't the > programming per-device ? Hello Viresh, I just posted this RFC as a real patch series: https://patchwork.kernel.org/project/linux-arm-msm/list/?series=142447 Note that I disregarded your review comment above, because this patch series only adds support for CPRv2, which is used in e.g. msm8916 and qcs404. There does not exist any QCOM SoC with CPRv2 for big little. For big little, there is CPRv3, which is very different from CPRv2. CPRv3 will require new and more complex DT bindings. Right now we don't even have plans to upstream a driver for CPRv3. Part of the reason is that CPR, for newer QCOM SoCs like sdm845, is now performed automatically by the Operating State Manager (OSM), for which we already have a kernel driver: drivers/cpufreq/qcom-cpufreq-hw.c Kind regards, Niklas
[PATCH 03/13] dt-bindings: cpufreq: qcom-nvmem: Make speedbin related properties optional
Not all Qualcomm platforms need to care about the speedbin efuse, nor the value blown into the speedbin efuse. Therefore, make the nvmem-cells and opp-supported-hw properties optional. Signed-off-by: Niklas Cassel --- Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt index 198441e80ba8..c5ea8b90e35d 100644 --- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt +++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt @@ -20,6 +20,10 @@ In 'cpus' nodes: In 'operating-points-v2' table: - compatible: Should be - 'operating-points-v2-kryo-cpu' for apq8096 and msm8996. + +Optional properties: + +In 'operating-points-v2' table: - nvmem-cells: A phandle pointing to a nvmem-cells node representing the efuse registers that has information about the speedbin that is used to select the right frequency/voltage -- 2.21.0
[PATCH 06/13] cpufreq: qcom: Add support for qcs404 on nvmem driver
Add support for qcs404 on nvmem driver.
The qcs404 SoC has support for Core Power Reduction (CPR), which is
implemented as a power domain provider, therefore add optional support
in this driver to attach to a genpd power domain.
Co-developed-by: Jorge Ramirez-Ortiz
Signed-off-by: Jorge Ramirez-Ortiz
Signed-off-by: Niklas Cassel
---
Changes since RFC:
-Remove empty stub.
-Add power domain attach as a feature in the match_data struct.
-Failing to attach to the power domain is treated as a hard error.
drivers/cpufreq/qcom-cpufreq-nvmem.c | 49 ++--
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c
b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index c0377b0eb2f4..ec87c14b598c 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -49,10 +50,12 @@ struct qcom_cpufreq_match_data {
int (*get_version)(struct device *cpu_dev,
struct nvmem_cell *speedbin_nvmem,
struct qcom_cpufreq_drv *drv);
+ const char **genpd_names;
};
struct qcom_cpufreq_drv {
struct opp_table **opp_tables;
+ struct opp_table **genpd_opp_tables;
u32 versions;
const struct qcom_cpufreq_match_data *data;
};
@@ -126,6 +129,12 @@ static const struct qcom_cpufreq_match_data
match_data_kryo = {
.get_version = qcom_cpufreq_kryo_name_version,
};
+static const char *qcs404_genpd_names[] = { "cpr", NULL };
+
+static const struct qcom_cpufreq_match_data match_data_qcs404 = {
+ .genpd_names = qcs404_genpd_names,
+};
+
static int qcom_cpufreq_probe(struct platform_device *pdev)
{
struct qcom_cpufreq_drv *drv;
@@ -188,11 +197,19 @@ static int qcom_cpufreq_probe(struct platform_device
*pdev)
goto free_drv;
}
+ drv->genpd_opp_tables = kcalloc(num_possible_cpus(),
+ sizeof(*drv->genpd_opp_tables),
+ GFP_KERNEL);
+ if (!drv->genpd_opp_tables) {
+ ret = -ENOMEM;
+ goto free_opp;
+ }
+
for_each_possible_cpu(cpu) {
cpu_dev = get_cpu_device(cpu);
if (NULL == cpu_dev) {
ret = -ENODEV;
- goto free_opp;
+ goto free_genpd_opp;
}
if (drv->data->get_version) {
@@ -203,7 +220,21 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
ret = PTR_ERR(drv->opp_tables[cpu]);
dev_err(cpu_dev,
"Failed to set supported hardware\n");
- goto free_opp;
+ goto free_genpd_opp;
+ }
+ }
+
+ if (drv->data->genpd_names) {
+ drv->genpd_opp_tables[cpu] =
+ dev_pm_opp_attach_genpd(cpu_dev,
+ drv->data->genpd_names);
+ if (IS_ERR(drv->genpd_opp_tables[cpu])) {
+ ret = PTR_ERR(drv->genpd_opp_tables[cpu]);
+ if (ret != -EPROBE_DEFER)
+ dev_err(cpu_dev,
+ "Could not attach to pm_domain:
%d\n",
+ ret);
+ goto free_genpd_opp;
}
}
}
@@ -218,6 +249,13 @@ static int qcom_cpufreq_probe(struct platform_device *pdev)
ret = PTR_ERR(cpufreq_dt_pdev);
dev_err(cpu_dev, "Failed to register platform device\n");
+free_genpd_opp:
+ for_each_possible_cpu(cpu) {
+ if (IS_ERR_OR_NULL(drv->genpd_opp_tables[cpu]))
+ break;
+ dev_pm_opp_detach_genpd(drv->genpd_opp_tables[cpu]);
+ }
+ kfree(drv->genpd_opp_tables);
free_opp:
for_each_possible_cpu(cpu) {
if (IS_ERR_OR_NULL(drv->opp_tables[cpu]))
@@ -238,11 +276,15 @@ static int qcom_cpufreq_remove(struct platform_device
*pdev)
platform_device_unregister(cpufreq_dt_pdev);
- for_each_possible_cpu(cpu)
+ for_each_possible_cpu(cpu) {
if (drv->opp_tables[cpu])
dev_pm_opp_put_supported_hw(drv->opp_tables[cpu]);
+ if (drv->genpd_opp_tables[cpu])
+ dev_pm_opp_detach_genpd(drv->genpd_opp_tables[cpu]);
+ }
kfree(drv->opp_tables);
+ kfree(drv->genpd_opp_tables);
kfree(drv);
return 0
[PATCH 05/13] dt-bindings: cpufreq: qcom-nvmem: Support pstates provided by a power domain
Some Qualcomm SoCs have support for Core Power Reduction (CPR).
On these platforms, we need to attach to the power domain provider
providing the performance states, so that the leaky device (the CPU)
can configure the performance states (which represent different
CPU clock frequencies).
Signed-off-by: Niklas Cassel
---
.../bindings/opp/qcom-nvmem-cpufreq.txt | 111 ++
1 file changed, 111 insertions(+)
diff --git a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
index c5ea8b90e35d..e19a95318e98 100644
--- a/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
+++ b/Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
@@ -23,6 +23,15 @@ In 'operating-points-v2' table:
Optional properties:
+In 'cpus' nodes:
+- power-domains: A phandle pointing to the PM domain specifier which provides
+ the performance states available for active state management.
+ Please refer to the power-domains bindings
+ Documentation/devicetree/bindings/power/power_domain.txt
+ and also examples below.
+- power-domain-names: Should be
+ - 'cpr' for qcs404.
+
In 'operating-points-v2' table:
- nvmem-cells: A phandle pointing to a nvmem-cells node representing the
efuse registers that has information about the
@@ -682,3 +691,105 @@ soc {
};
};
};
+
+Example 2:
+-
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ CPU0: cpu@100 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x100>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU1: cpu@101 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x101>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU2: cpu@102 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x102>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+
+ CPU3: cpu@103 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x103>;
+
+ clocks = <_glb>;
+ operating-points-v2 = <_opp_table>;
+ power-domains = <>;
+ power-domain-names = "cpr";
+ };
+ };
+
+ cpu_opp_table: cpu-opp-table {
+ compatible = "operating-points-v2-kryo-cpu";
+ opp-shared;
+
+ opp-109440 {
+ opp-hz = /bits/ 64 <109440>;
+ required-opps = <_opp1>;
+ };
+ opp-124800 {
+ opp-hz = /bits/ 64 <124800>;
+ required-opps = <_opp2>;
+ };
+ opp-140160 {
+ opp-hz = /bits/ 64 <140160>;
+ required-opps = <_opp3>;
+ };
+ };
+
+ cpr_opp_table: cpr-opp-table {
+ compatible = "operating-points-v2-qcom-level";
+
+ cpr_opp1: opp1 {
+ opp-level = <1>;
+
+ };
+ cpr_opp2: opp2 {
+ opp-level = <2>;
+
+ };
+ cpr_opp3: opp3 {
+ opp-level = <3>;
+
+ };
+ };
+
+
+
+soc {
+
+ cprpd: cpr@b018000 {
+ compatible = "qcom,qcs404-cpr", "qcom,cpr";
+ reg = <0x0b018000 0x1000>;
+
+ vdd-apc-supply = <_s3>;
+ #power-domain-cells = <0>;
+ operating-points-v2 = <_opp_table>;
+
+ };
+};
--
2.21.0
[PATCH 11/13] arm64: dts: qcom: qcs404: Add CPR and populate OPP table
Add CPR and populate OPP table.
Co-developed-by: Jorge Ramirez-Ortiz
Signed-off-by: Jorge Ramirez-Ortiz
Signed-off-by: Niklas Cassel
---
arch/arm64/boot/dts/qcom/qcs404.dtsi | 145 +--
1 file changed, 137 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi
b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index ff9198740431..5b6276c3ec42 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -38,7 +38,8 @@
#cooling-cells = <2>;
clocks = <_glb>;
operating-points-v2 = <_opp_table>;
- cpu-supply = <_s3>;
+ power-domains = <>;
+ power-domain-names = "cpr";
};
CPU1: cpu@101 {
@@ -51,7 +52,8 @@
#cooling-cells = <2>;
clocks = <_glb>;
operating-points-v2 = <_opp_table>;
- cpu-supply = <_s3>;
+ power-domains = <>;
+ power-domain-names = "cpr";
};
CPU2: cpu@102 {
@@ -64,7 +66,8 @@
#cooling-cells = <2>;
clocks = <_glb>;
operating-points-v2 = <_opp_table>;
- cpu-supply = <_s3>;
+ power-domains = <>;
+ power-domain-names = "cpr";
};
CPU3: cpu@103 {
@@ -77,7 +80,8 @@
#cooling-cells = <2>;
clocks = <_glb>;
operating-points-v2 = <_opp_table>;
- cpu-supply = <_s3>;
+ power-domains = <>;
+ power-domain-names = "cpr";
};
L2_0: l2-cache {
@@ -101,20 +105,40 @@
};
cpu_opp_table: cpu-opp-table {
- compatible = "operating-points-v2";
+ compatible = "operating-points-v2-kryo-cpu";
opp-shared;
opp-109440 {
opp-hz = /bits/ 64 <109440>;
- opp-microvolt = <1224000 1224000 1224000>;
+ required-opps = <_opp1>;
};
opp-124800 {
opp-hz = /bits/ 64 <124800>;
- opp-microvolt = <1288000 1288000 1288000>;
+ required-opps = <_opp2>;
};
opp-140160 {
opp-hz = /bits/ 64 <140160>;
- opp-microvolt = <1384000 1384000 1384000>;
+ required-opps = <_opp3>;
+ };
+ };
+
+ cpr_opp_table: cpr-opp-table {
+ compatible = "operating-points-v2-qcom-level";
+
+ cpr_opp1: opp1 {
+ opp-level = <1>;
+ qcom,opp-fuse-level = <1>;
+ opp-hz = /bits/ 64 <109440>;
+ };
+ cpr_opp2: opp2 {
+ opp-level = <2>;
+ qcom,opp-fuse-level = <2>;
+ opp-hz = /bits/ 64 <124800>;
+ };
+ cpr_opp3: opp3 {
+ opp-level = <3>;
+ qcom,opp-fuse-level = <3>;
+ opp-hz = /bits/ 64 <140160>;
};
};
@@ -294,6 +318,62 @@
tsens_caldata: caldata@d0 {
reg = <0x1f8 0x14>;
};
+ cpr_efuse_speedbin: speedbin@13c {
+ reg = <0x13c 0x4>;
+ bits = <2 3>;
+ };
+ cpr_efuse_quot_offset1: qoffset1@231 {
+ reg = <0x231 0x4>;
+ bits = <4 7>;
+ };
+ cpr_efuse_quot_offset2: qoffset2@232 {
+ reg = <0x232 0x4>;
+ bits = <3 7>;
+ };
+ cpr_efuse_quot_offset3: qoffset3@233 {
+ reg = <0x233 0x4>;
+ bits = <2 7>;
+ };
+ cpr_efuse_init_voltage1: ivoltage1@229 {
+ reg = <0x229 0x4>;
+ bits = <4 6>;
+
[PATCH 07/13] cpufreq: Add qcs404 to cpufreq-dt-platdev blacklist
From: Jorge Ramirez-Ortiz
Add qcs404 to cpufreq-dt-platdev blacklist.
Signed-off-by: Jorge Ramirez-Ortiz
Co-developed-by: Niklas Cassel
Signed-off-by: Niklas Cassel
---
drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c
b/drivers/cpufreq/cpufreq-dt-platdev.c
index 03dc4244ab00..ec6ef996e637 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -123,6 +123,7 @@ static const struct of_device_id blacklist[] __initconst = {
{ .compatible = "qcom,apq8096", },
{ .compatible = "qcom,msm8996", },
+ { .compatible = "qcom,qcs404", },
{ .compatible = "st,stih407", },
{ .compatible = "st,stih410", },
--
2.21.0
[PATCH 12/13] arm64: defconfig: enable CONFIG_QCOM_CPR
Enable CONFIG_QCOM_CPR. Signed-off-by: Niklas Cassel --- arch/arm64/configs/defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index bfadf18e71c2..d1e8ad5d3079 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -416,6 +416,7 @@ CONFIG_GPIO_PCA953X_IRQ=y CONFIG_GPIO_MAX77620=y CONFIG_POWER_AVS=y CONFIG_ROCKCHIP_IODOMAIN=y +CONFIG_QCOM_CPR=y CONFIG_POWER_RESET_MSM=y CONFIG_POWER_RESET_XGENE=y CONFIG_POWER_RESET_SYSCON=y -- 2.21.0
