Re: Financial Aid
I, Mikhail Fridman have selected you specifically as one of my beneficiaries for my Charitable Donation of $5 Million Dollars, Check the link below for confirmation: https://www.rt.com/business/343781-mikhail-fridman-will-charity/ I await your earliest response for further directives. Best Regards, Mikhail Fridman. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] media: imx: Don't register IPU subdevs/links if CSI port missing
The second IPU internal sub-devices were being registered and links
to them created even when the second IPU is not present. This is wrong
for i.MX6 S/DL and i.MX53 which have only a single IPU.
Fixes: e130291212df5 ("[media] media: Add i.MX media core driver")
Signed-off-by: Steve Longerbeam
Cc: sta...@vger.kernel.org
---
drivers/staging/media/imx/imx-media-dev.c | 7 --
.../staging/media/imx/imx-media-internal-sd.c | 22 +-
drivers/staging/media/imx/imx-media-of.c | 76 ---
drivers/staging/media/imx/imx-media.h | 3 +-
4 files changed, 53 insertions(+), 55 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-dev.c
b/drivers/staging/media/imx/imx-media-dev.c
index edf9c80bbbc8..924166cf957b 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -487,13 +487,6 @@ static int imx_media_probe(struct platform_device *pdev)
goto notifier_cleanup;
}
- ret = imx_media_add_ipu_internal_subdevs(imxmd);
- if (ret) {
- v4l2_err(>v4l2_dev,
-"add_ipu_internal_subdevs failed with %d\n", ret);
- goto notifier_cleanup;
- }
-
/* no subdevs? just bail */
if (list_empty(>notifier.asd_list)) {
ret = -ENODEV;
diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c
b/drivers/staging/media/imx/imx-media-internal-sd.c
index b8e763dbbecb..3811c12d475b 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -298,13 +298,14 @@ static int add_internal_subdev(struct imx_media_dev
*imxmd,
}
/* adds the internal subdevs in one ipu */
-static int add_ipu_internal_subdevs(struct imx_media_dev *imxmd, int ipu_id)
+int imx_media_add_ipu_internal_subdevs(struct imx_media_dev *imxmd,
+ int ipu_id)
{
enum isd_enum i;
+ int ret;
for (i = 0; i < num_isd; i++) {
const struct internal_subdev *isd = _subdev[i];
- int ret;
/*
* the CSIs are represented in the device-tree, so those
@@ -322,25 +323,10 @@ static int add_ipu_internal_subdevs(struct imx_media_dev
*imxmd, int ipu_id)
}
if (ret)
- return ret;
+ goto remove;
}
return 0;
-}
-
-int imx_media_add_ipu_internal_subdevs(struct imx_media_dev *imxmd)
-{
- int ret;
-
- ret = add_ipu_internal_subdevs(imxmd, 0);
- if (ret)
- goto remove;
-
- ret = add_ipu_internal_subdevs(imxmd, 1);
- if (ret)
- goto remove;
-
- return 0;
remove:
imx_media_remove_ipu_internal_subdevs(imxmd);
diff --git a/drivers/staging/media/imx/imx-media-of.c
b/drivers/staging/media/imx/imx-media-of.c
index 2da81a5af274..32ed1a99813f 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -20,50 +20,68 @@
#include
#include "imx-media.h"
-static int of_add_csi(struct imx_media_dev *imxmd, struct device_node *csi_np)
-{
- int ret;
-
- if (!of_device_is_available(csi_np)) {
- dev_dbg(imxmd->md.dev, "%s: %pOFn not enabled\n", __func__,
- csi_np);
- /* unavailable is not an error */
- return 0;
- }
-
- /* add CSI fwnode to async notifier */
- ret = imx_media_add_async_subdev(imxmd, of_fwnode_handle(csi_np), NULL);
- if (ret) {
- if (ret == -EEXIST) {
- /* already added, everything is fine */
- return 0;
- }
-
- /* other error, can't continue */
- return ret;
- }
-
- return 0;
-}
-
int imx_media_add_of_subdevs(struct imx_media_dev *imxmd,
struct device_node *np)
{
+ bool ipu_found[2] = {false, false};
struct device_node *csi_np;
int i, ret;
+ u32 ipu_id;
for (i = 0; ; i++) {
csi_np = of_parse_phandle(np, "ports", i);
if (!csi_np)
break;
- ret = of_add_csi(imxmd, csi_np);
+ if (!of_device_is_available(csi_np)) {
+ /* ignore this CSI if not available */
+ of_node_put(csi_np);
+ continue;
+ }
+
+ /* add CSI fwnode to async notifier */
+ ret = imx_media_add_async_subdev(imxmd,
+of_fwnode_handle(csi_np),
+NULL);
+ if (ret) {
+ if (ret == -EEXIST) {
+ /* already added, everything is fine */
+ of_node_put(csi_np);
+
[PATCH 3/4] media: imx: Rename functions that add IPU-internal subdevs/links
For the functions that add and remove the internal IPU subdevice
descriptors and links between them, rename them to make clear they
are the subdevs and links internal to the IPU. Also rename the
platform data structure for the internal IPU subdevices.
No functional changes.
Signed-off-by: Steve Longerbeam
Cc: sta...@vger.kernel.org
---
drivers/staging/media/imx/imx-ic-common.c| 2 +-
drivers/staging/media/imx/imx-media-dev.c| 10 +-
.../staging/media/imx/imx-media-internal-sd.c| 16
drivers/staging/media/imx/imx-media-vdic.c | 2 +-
drivers/staging/media/imx/imx-media.h| 10 +-
5 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-common.c
b/drivers/staging/media/imx/imx-ic-common.c
index cfdd4900a3be..94dc5d88a069 100644
--- a/drivers/staging/media/imx/imx-ic-common.c
+++ b/drivers/staging/media/imx/imx-ic-common.c
@@ -26,7 +26,7 @@ static struct imx_ic_ops *ic_ops[IC_NUM_OPS] = {
static int imx_ic_probe(struct platform_device *pdev)
{
- struct imx_media_internal_sd_platformdata *pdata;
+ struct imx_media_ipu_internal_sd_pdata *pdata;
struct imx_ic_priv *priv;
int ret;
diff --git a/drivers/staging/media/imx/imx-media-dev.c
b/drivers/staging/media/imx/imx-media-dev.c
index 4b344a4a3706..edf9c80bbbc8 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -155,7 +155,7 @@ static int imx_media_create_links(struct
v4l2_async_notifier *notifier)
case IMX_MEDIA_GRP_ID_IC_PRPVF:
case IMX_MEDIA_GRP_ID_CSI0:
case IMX_MEDIA_GRP_ID_CSI1:
- ret = imx_media_create_internal_links(imxmd, sd);
+ ret = imx_media_create_ipu_internal_links(imxmd, sd);
if (ret)
return ret;
/*
@@ -487,10 +487,10 @@ static int imx_media_probe(struct platform_device *pdev)
goto notifier_cleanup;
}
- ret = imx_media_add_internal_subdevs(imxmd);
+ ret = imx_media_add_ipu_internal_subdevs(imxmd);
if (ret) {
v4l2_err(>v4l2_dev,
-"add_internal_subdevs failed with %d\n", ret);
+"add_ipu_internal_subdevs failed with %d\n", ret);
goto notifier_cleanup;
}
@@ -513,7 +513,7 @@ static int imx_media_probe(struct platform_device *pdev)
return 0;
del_int:
- imx_media_remove_internal_subdevs(imxmd);
+ imx_media_remove_ipu_internal_subdevs(imxmd);
notifier_cleanup:
v4l2_async_notifier_cleanup(>notifier);
v4l2_device_unregister(>v4l2_dev);
@@ -530,7 +530,7 @@ static int imx_media_remove(struct platform_device *pdev)
v4l2_info(>v4l2_dev, "Removing imx-media\n");
v4l2_async_notifier_unregister(>notifier);
- imx_media_remove_internal_subdevs(imxmd);
+ imx_media_remove_ipu_internal_subdevs(imxmd);
v4l2_async_notifier_cleanup(>notifier);
v4l2_device_unregister(>v4l2_dev);
media_device_unregister(>md);
diff --git a/drivers/staging/media/imx/imx-media-internal-sd.c
b/drivers/staging/media/imx/imx-media-internal-sd.c
index 0fdc45dbfb76..b8e763dbbecb 100644
--- a/drivers/staging/media/imx/imx-media-internal-sd.c
+++ b/drivers/staging/media/imx/imx-media-internal-sd.c
@@ -1,7 +1,7 @@
/*
* Media driver for Freescale i.MX5/6 SOC
*
- * Adds the internal subdevices and the media links between them.
+ * Adds the IPU internal subdevices and the media links between them.
*
* Copyright (c) 2016 Mentor Graphics Inc.
*
@@ -192,7 +192,7 @@ static struct v4l2_subdev *find_sink(struct imx_media_dev
*imxmd,
/*
* retrieve IPU id from subdev name, note: can't get this from
-* struct imx_media_internal_sd_platformdata because if src is
+* struct imx_media_ipu_internal_sd_pdata because if src is
* a CSI, it has different struct ipu_client_platformdata which
* does not contain IPU id.
*/
@@ -229,8 +229,8 @@ static int create_ipu_internal_link(struct imx_media_dev
*imxmd,
return ret;
}
-int imx_media_create_internal_links(struct imx_media_dev *imxmd,
- struct v4l2_subdev *sd)
+int imx_media_create_ipu_internal_links(struct imx_media_dev *imxmd,
+ struct v4l2_subdev *sd)
{
const struct internal_subdev *intsd;
const struct internal_pad *intpad;
@@ -270,7 +270,7 @@ static int add_internal_subdev(struct imx_media_dev *imxmd,
const struct internal_subdev *isd,
int ipu_id)
{
- struct imx_media_internal_sd_platformdata pdata;
+ struct imx_media_ipu_internal_sd_pdata pdata;
struct platform_device_info pdevinfo = {};
struct platform_device
[PATCH 2/4] media: imx: Clear fwnode link struct for each endpoint iteration
In imx_media_create_csi_of_links(), the 'struct v4l2_fwnode_link' must
be cleared for each endpoint iteration, otherwise if the remote port
has no "reg" property, link.remote_port will not be reset to zero.
This was discovered on the i.MX53 SMD board, since the OV5642 connects
directly to ipu1_csi0 and has a single source port with no "reg"
property.
Fixes: 621b08eabcddb ("media: staging/imx: remove static media link arrays")
Signed-off-by: Steve Longerbeam
Cc: sta...@vger.kernel.org
---
drivers/staging/media/imx/imx-media-of.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-of.c
b/drivers/staging/media/imx/imx-media-of.c
index a01327f6e045..2da81a5af274 100644
--- a/drivers/staging/media/imx/imx-media-of.c
+++ b/drivers/staging/media/imx/imx-media-of.c
@@ -143,15 +143,18 @@ int imx_media_create_csi_of_links(struct imx_media_dev
*imxmd,
struct v4l2_subdev *csi)
{
struct device_node *csi_np = csi->dev->of_node;
- struct fwnode_handle *fwnode, *csi_ep;
- struct v4l2_fwnode_link link;
struct device_node *ep;
- int ret;
-
- link.local_node = of_fwnode_handle(csi_np);
- link.local_port = CSI_SINK_PAD;
for_each_child_of_node(csi_np, ep) {
+ struct fwnode_handle *fwnode, *csi_ep;
+ struct v4l2_fwnode_link link;
+ int ret;
+
+ memset(, 0, sizeof(link));
+
+ link.local_node = of_fwnode_handle(csi_np);
+ link.local_port = CSI_SINK_PAD;
+
csi_ep = of_fwnode_handle(ep);
fwnode = fwnode_graph_get_remote_endpoint(csi_ep);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] media: imx: csi: Allow unknown nearest upstream entities
On i.MX6, the nearest upstream entity to the CSI can only be the
CSI video muxes or the Synopsys DW MIPI CSI-2 receiver.
However the i.MX53 has no CSI video muxes or a MIPI CSI-2 receiver.
So allow for the nearest upstream entity to the CSI to be something
other than those.
Fixes: bf3cfaa712e5c ("media: staging/imx: get CSI bus type from nearest
upstream entity")
Signed-off-by: Steve Longerbeam
Cc: sta...@vger.kernel.org
---
drivers/staging/media/imx/imx-media-csi.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 555aa45e02e3..b9af7d3d4974 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -154,9 +154,10 @@ static inline bool requires_passthrough(struct
v4l2_fwnode_endpoint *ep,
/*
* Parses the fwnode endpoint from the source pad of the entity
* connected to this CSI. This will either be the entity directly
- * upstream from the CSI-2 receiver, or directly upstream from the
- * video mux. The endpoint is needed to determine the bus type and
- * bus config coming into the CSI.
+ * upstream from the CSI-2 receiver, directly upstream from the
+ * video mux, or directly upstream from the CSI itself. The endpoint
+ * is needed to determine the bus type and bus config coming into
+ * the CSI.
*/
static int csi_get_upstream_endpoint(struct csi_priv *priv,
struct v4l2_fwnode_endpoint *ep)
@@ -172,7 +173,8 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
if (!priv->src_sd)
return -EPIPE;
- src = >src_sd->entity;
+ sd = priv->src_sd;
+ src = >entity;
if (src->function == MEDIA_ENT_F_VID_MUX) {
/*
@@ -186,6 +188,14 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
src = >entity;
}
+ /*
+* If the source is neither the video mux nor the CSI-2 receiver,
+* get the source pad directly upstream from CSI itself.
+*/
+ if (src->function != MEDIA_ENT_F_VID_MUX &&
+ sd->grp_id != IMX_MEDIA_GRP_ID_CSI2)
+ src = >sd.entity;
+
/* get source pad of entity directly upstream from src */
pad = imx_media_find_upstream_pad(priv->md, src, 0);
if (IS_ERR(pad))
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/3] adt7316 regmap implementation
On Sat, 2019-01-19 at 19:04 +, Jonathan Cameron wrote: > On Sun, 20 Jan 2019 00:09:29 +0530 > Shreeya Patel wrote: > > > This patchset consist of some initial patches for heading > > towards the regmap implementation and also the final patch > > which enables the driver to use regmap API thus removing > > the redundant and common code. > > > > Shreeya Patel (3): > > Staging: iio: adt7316: Remove irq from bus structure > > Staging: iio: adt7316: Remove multi read and write functions > > Staging: iio: adt7316: Add regmap support > > > > drivers/staging/iio/addac/adt7316-i2c.c | 101 ++-- > > drivers/staging/iio/addac/adt7316-spi.c | 95 +++ > > drivers/staging/iio/addac/adt7316.c | 147 -- > > -- > > drivers/staging/iio/addac/adt7316.h | 15 +-- > > 4 files changed, 103 insertions(+), 255 deletions(-) > > > > Hi Shreeya, > > I'm not seeing a change log for any of these. Please make sure to > add one so we know what changed since V1. Saves a lot of trying > to remember things from a while back! > Sorry, I forgot to add it. I remembered it when I had already sent the patches. I'll also cc Jeremy in V3. Thanks > Thanks, > > Jonathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/3] adt7316 regmap implementation
On Sun, 20 Jan 2019 00:09:29 +0530 Shreeya Patel wrote: > This patchset consist of some initial patches for heading > towards the regmap implementation and also the final patch > which enables the driver to use regmap API thus removing > the redundant and common code. > > Shreeya Patel (3): > Staging: iio: adt7316: Remove irq from bus structure > Staging: iio: adt7316: Remove multi read and write functions > Staging: iio: adt7316: Add regmap support > > drivers/staging/iio/addac/adt7316-i2c.c | 101 ++-- > drivers/staging/iio/addac/adt7316-spi.c | 95 +++ > drivers/staging/iio/addac/adt7316.c | 147 > drivers/staging/iio/addac/adt7316.h | 15 +-- > 4 files changed, 103 insertions(+), 255 deletions(-) > Hi Shreeya, I'm not seeing a change log for any of these. Please make sure to add one so we know what changed since V1. Saves a lot of trying to remember things from a while back! Thanks, Jonathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] Staging: iio: adt7316: Remove irq from bus structure
On Sun, 20 Jan 2019 00:09:30 +0530
Shreeya Patel wrote:
> interrupt request is not needed to be present in the bus
> structure. It is a good option to pass it as a parameter
> in the probe function instead of having it in the bus structure.
>
> Signed-off-by: Shreeya Patel
Hi Shreeya,
I just tried applying this to the current testing branch of
iio.git and unfortunately it doesn't apply. Please rebase
on that as there have been various changes.
Superficially I think it is all white space related and a bit
of code movement, but I'd rather you made sure the rebase
worked well than I made assumptions!
The change looks good otherwise.
Jonathan
> ---
> drivers/staging/iio/addac/adt7316-i2c.c | 3 +--
> drivers/staging/iio/addac/adt7316-spi.c | 4 ++--
> drivers/staging/iio/addac/adt7316.c | 15 +++
> drivers/staging/iio/addac/adt7316.h | 3 +--
> 4 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/iio/addac/adt7316-i2c.c
> b/drivers/staging/iio/addac/adt7316-i2c.c
> index ac91163656b5..335c17731cf7 100644
> --- a/drivers/staging/iio/addac/adt7316-i2c.c
> +++ b/drivers/staging/iio/addac/adt7316-i2c.c
> @@ -103,14 +103,13 @@ static int adt7316_i2c_probe(struct i2c_client *client,
> {
> struct adt7316_bus bus = {
> .client = client,
> - .irq = client->irq,
> .read = adt7316_i2c_read,
> .write = adt7316_i2c_write,
> .multi_read = adt7316_i2c_multi_read,
> .multi_write = adt7316_i2c_multi_write,
> };
>
> - return adt7316_probe(>dev, , id->name);
> + return adt7316_probe(>dev, , id->name, client->irq);
> }
>
> static const struct i2c_device_id adt7316_i2c_id[] = {
> diff --git a/drivers/staging/iio/addac/adt7316-spi.c
> b/drivers/staging/iio/addac/adt7316-spi.c
> index e75827e326a6..adaaa3eecd04 100644
> --- a/drivers/staging/iio/addac/adt7316-spi.c
> +++ b/drivers/staging/iio/addac/adt7316-spi.c
> @@ -93,7 +93,6 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
> {
> struct adt7316_bus bus = {
> .client = spi_dev,
> - .irq = spi_dev->irq,
> .read = adt7316_spi_read,
> .write = adt7316_spi_write,
> .multi_read = adt7316_spi_multi_read,
> @@ -112,7 +111,8 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
> adt7316_spi_write(spi_dev, 0, 0);
> adt7316_spi_write(spi_dev, 0, 0);
>
> - return adt7316_probe(_dev->dev, , spi_dev->modalias);
> + return adt7316_probe(_dev->dev, , spi_dev->modalias,
> + spi_dev->irq);
> }
>
> static const struct spi_device_id adt7316_spi_id[] = {
> diff --git a/drivers/staging/iio/addac/adt7316.c
> b/drivers/staging/iio/addac/adt7316.c
> index 1ca4ee0f30ee..6b4b80fd80cc 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -1807,12 +1807,12 @@ static irqreturn_t adt7316_event_handler(int irq,
> void *private)
> return IRQ_HANDLED;
> }
>
> -static int adt7316_setup_irq(struct iio_dev *indio_dev)
> +static int adt7316_setup_irq(struct iio_dev *indio_dev, int irq)
> {
> struct adt7316_chip_info *chip = iio_priv(indio_dev);
> int irq_type, ret;
>
> - irq_type = irqd_get_trigger_type(irq_get_irq_data(chip->bus.irq));
> + irq_type = irqd_get_trigger_type(irq_get_irq_data(irq));
>
> switch (irq_type) {
> case IRQF_TRIGGER_HIGH:
> @@ -1828,13 +1828,12 @@ static int adt7316_setup_irq(struct iio_dev
> *indio_dev)
> break;
> }
>
> - ret = devm_request_threaded_irq(_dev->dev, chip->bus.irq,
> + ret = devm_request_threaded_irq(_dev->dev, irq,
> NULL, adt7316_event_handler,
> irq_type | IRQF_ONESHOT,
> indio_dev->name, indio_dev);
> if (ret) {
> - dev_err(_dev->dev, "failed to request irq %d\n",
> - chip->bus.irq);
> + dev_err(_dev->dev, "failed to request irq %d\n", irq);
> return ret;
> }
>
> @@ -2134,7 +2133,7 @@ static const struct iio_info adt7516_info = {
> * device probe and remove
> */
> int adt7316_probe(struct device *dev, struct adt7316_bus *bus,
> - const char *name)
> + const char *name, int irq)
> {
> struct adt7316_chip_info *chip;
> struct iio_dev *indio_dev;
> @@ -2180,8 +2179,8 @@ int adt7316_probe(struct device *dev, struct
> adt7316_bus *bus,
> indio_dev->name = name;
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> - if (chip->bus.irq > 0) {
> - ret = adt7316_setup_irq(indio_dev);
> + if (irq > 0) {
> + ret = adt7316_setup_irq(indio_dev, irq);
> if (ret)
> return ret;
> }
> diff --git a/drivers/staging/iio/addac/adt7316.h
>
[PATCH v2 2/3] Staging: iio: adt7316: Remove multi read and write functions
Currently, adt7316 doesn't use multi read and multi write
functions hence remove the redundant code and make the
necessary changes in the code.
Signed-off-by: Shreeya Patel
---
drivers/staging/iio/addac/adt7316-i2c.c | 40 -
drivers/staging/iio/addac/adt7316-spi.c | 31 ---
drivers/staging/iio/addac/adt7316.h | 2 --
3 files changed, 6 insertions(+), 67 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316-i2c.c
b/drivers/staging/iio/addac/adt7316-i2c.c
index 335c17731cf7..40aa9e2f1966 100644
--- a/drivers/staging/iio/addac/adt7316-i2c.c
+++ b/drivers/staging/iio/addac/adt7316-i2c.c
@@ -56,44 +56,6 @@ static int adt7316_i2c_write(void *client, u8 reg, u8 data)
return ret;
}
-static int adt7316_i2c_multi_read(void *client, u8 reg, u8 count, u8 *data)
-{
- struct i2c_client *cl = client;
- int i, ret = 0;
-
- if (count > ADT7316_REG_MAX_ADDR)
- count = ADT7316_REG_MAX_ADDR;
-
- for (i = 0; i < count; i++) {
- ret = adt7316_i2c_read(cl, reg, [i]);
- if (ret < 0) {
- dev_err(>dev, "I2C multi read error\n");
- return ret;
- }
- }
-
- return 0;
-}
-
-static int adt7316_i2c_multi_write(void *client, u8 reg, u8 count, u8 *data)
-{
- struct i2c_client *cl = client;
- int i, ret = 0;
-
- if (count > ADT7316_REG_MAX_ADDR)
- count = ADT7316_REG_MAX_ADDR;
-
- for (i = 0; i < count; i++) {
- ret = adt7316_i2c_write(cl, reg, data[i]);
- if (ret < 0) {
- dev_err(>dev, "I2C multi write error\n");
- return ret;
- }
- }
-
- return 0;
-}
-
/*
* device probe and remove
*/
@@ -105,8 +67,6 @@ static int adt7316_i2c_probe(struct i2c_client *client,
.client = client,
.read = adt7316_i2c_read,
.write = adt7316_i2c_write,
- .multi_read = adt7316_i2c_multi_read,
- .multi_write = adt7316_i2c_multi_write,
};
return adt7316_probe(>dev, , id->name, client->irq);
diff --git a/drivers/staging/iio/addac/adt7316-spi.c
b/drivers/staging/iio/addac/adt7316-spi.c
index adaaa3eecd04..1a78dc1e2840 100644
--- a/drivers/staging/iio/addac/adt7316-spi.c
+++ b/drivers/staging/iio/addac/adt7316-spi.c
@@ -23,15 +23,12 @@
* adt7316 register access by SPI
*/
-static int adt7316_spi_multi_read(void *client, u8 reg, u8 count, u8 *data)
+static int adt7316_spi_read(void *client, u8 reg, u8 *data)
{
struct spi_device *spi_dev = client;
u8 cmd[2];
int ret = 0;
- if (count > ADT7316_REG_MAX_ADDR)
- count = ADT7316_REG_MAX_ADDR;
-
cmd[0] = ADT7316_SPI_CMD_WRITE;
cmd[1] = reg;
@@ -43,7 +40,7 @@ static int adt7316_spi_multi_read(void *client, u8 reg, u8
count, u8 *data)
cmd[0] = ADT7316_SPI_CMD_READ;
- ret = spi_write_then_read(spi_dev, cmd, 1, data, count);
+ ret = spi_write_then_read(spi_dev, cmd, 1, data, 1);
if (ret < 0) {
dev_err(_dev->dev, "SPI read data error\n");
return ret;
@@ -52,21 +49,17 @@ static int adt7316_spi_multi_read(void *client, u8 reg, u8
count, u8 *data)
return 0;
}
-static int adt7316_spi_multi_write(void *client, u8 reg, u8 count, u8 *data)
+static int adt7316_spi_write(void *client, u8 reg, u8 val)
{
struct spi_device *spi_dev = client;
u8 buf[ADT7316_REG_MAX_ADDR + 2];
- int i, ret = 0;
-
- if (count > ADT7316_REG_MAX_ADDR)
- count = ADT7316_REG_MAX_ADDR;
+ int ret = 0;
buf[0] = ADT7316_SPI_CMD_WRITE;
buf[1] = reg;
- for (i = 0; i < count; i++)
- buf[i + 2] = data[i];
+ buf[2] = val;
- ret = spi_write(spi_dev, buf, count + 2);
+ ret = spi_write(spi_dev, buf, 3);
if (ret < 0) {
dev_err(_dev->dev, "SPI write error\n");
return ret;
@@ -75,16 +68,6 @@ static int adt7316_spi_multi_write(void *client, u8 reg, u8
count, u8 *data)
return ret;
}
-static int adt7316_spi_read(void *client, u8 reg, u8 *data)
-{
- return adt7316_spi_multi_read(client, reg, 1, data);
-}
-
-static int adt7316_spi_write(void *client, u8 reg, u8 val)
-{
- return adt7316_spi_multi_write(client, reg, 1, );
-}
-
/*
* device probe and remove
*/
@@ -95,8 +78,6 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
.client = spi_dev,
.read = adt7316_spi_read,
.write = adt7316_spi_write,
- .multi_read = adt7316_spi_multi_read,
- .multi_write = adt7316_spi_multi_write,
};
/* don't exceed max specified SPI CLK frequency */
diff --git a/drivers/staging/iio/addac/adt7316.h
b/drivers/staging/iio/addac/adt7316.h
index
[PATCH v2 3/3] Staging: iio: adt7316: Add regmap support
Both i2c and spi drivers have functions for reading and writing
to/from registers. Remove this redundant and common code by using
regmap API.
Signed-off-by: Shreeya Patel
---
drivers/staging/iio/addac/adt7316-i2c.c | 60 +++
drivers/staging/iio/addac/adt7316-spi.c | 74 +++--
drivers/staging/iio/addac/adt7316.c | 132
drivers/staging/iio/addac/adt7316.h | 10 +-
4 files changed, 94 insertions(+), 182 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316-i2c.c
b/drivers/staging/iio/addac/adt7316-i2c.c
index 40aa9e2f1966..435b65845174 100644
--- a/drivers/staging/iio/addac/adt7316-i2c.c
+++ b/drivers/staging/iio/addac/adt7316-i2c.c
@@ -12,64 +12,28 @@
#include
#include
#include
+#include
#include "adt7316.h"
-/*
- * adt7316 register access by I2C
- */
-static int adt7316_i2c_read(void *client, u8 reg, u8 *data)
-{
- struct i2c_client *cl = client;
- int ret;
-
- ret = i2c_smbus_write_byte(cl, reg);
- if (ret < 0) {
- dev_err(>dev, "I2C fail to select reg\n");
- return ret;
- }
-
- ret = i2c_smbus_read_byte(client);
-
- if (!ret)
- return -EIO;
-
- if (ret < 0) {
- dev_err(>dev, "I2C read error\n");
- return ret;
- }
-
- *data = ret;
-
- return 0;
-}
-
-static int adt7316_i2c_write(void *client, u8 reg, u8 data)
-{
- struct i2c_client *cl = client;
- int ret = 0;
-
- ret = i2c_smbus_write_byte_data(cl, reg, data);
- if (ret < 0)
- dev_err(>dev, "I2C write error\n");
-
- return ret;
-}
-
/*
* device probe and remove
*/
-
static int adt7316_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- struct adt7316_bus bus = {
- .client = client,
- .read = adt7316_i2c_read,
- .write = adt7316_i2c_write,
- };
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_i2c(client, _regmap_config);
+
+ if (IS_ERR(regmap)) {
+ dev_err(>dev, "Error initializing i2c regmap: %ld\n",
+ PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
- return adt7316_probe(>dev, , id->name, client->irq);
+ return adt7316_probe(>dev, regmap, id ? id->name : NULL,
+client->irq);
}
static const struct i2c_device_id adt7316_i2c_id[] = {
diff --git a/drivers/staging/iio/addac/adt7316-spi.c
b/drivers/staging/iio/addac/adt7316-spi.c
index 1a78dc1e2840..203b5c3ada6e 100644
--- a/drivers/staging/iio/addac/adt7316-spi.c
+++ b/drivers/staging/iio/addac/adt7316-spi.c
@@ -11,74 +11,19 @@
#include
#include
#include
+#include
#include
#include "adt7316.h"
#define ADT7316_SPI_MAX_FREQ_HZ500
-#define ADT7316_SPI_CMD_READ 0x91
-#define ADT7316_SPI_CMD_WRITE 0x90
-
-/*
- * adt7316 register access by SPI
- */
-
-static int adt7316_spi_read(void *client, u8 reg, u8 *data)
-{
- struct spi_device *spi_dev = client;
- u8 cmd[2];
- int ret = 0;
-
- cmd[0] = ADT7316_SPI_CMD_WRITE;
- cmd[1] = reg;
-
- ret = spi_write(spi_dev, cmd, 2);
- if (ret < 0) {
- dev_err(_dev->dev, "SPI fail to select reg\n");
- return ret;
- }
-
- cmd[0] = ADT7316_SPI_CMD_READ;
-
- ret = spi_write_then_read(spi_dev, cmd, 1, data, 1);
- if (ret < 0) {
- dev_err(_dev->dev, "SPI read data error\n");
- return ret;
- }
-
- return 0;
-}
-
-static int adt7316_spi_write(void *client, u8 reg, u8 val)
-{
- struct spi_device *spi_dev = client;
- u8 buf[ADT7316_REG_MAX_ADDR + 2];
- int ret = 0;
-
- buf[0] = ADT7316_SPI_CMD_WRITE;
- buf[1] = reg;
- buf[2] = val;
-
- ret = spi_write(spi_dev, buf, 3);
- if (ret < 0) {
- dev_err(_dev->dev, "SPI write error\n");
- return ret;
- }
-
- return ret;
-}
/*
* device probe and remove
*/
-
static int adt7316_spi_probe(struct spi_device *spi_dev)
{
- struct adt7316_bus bus = {
- .client = spi_dev,
- .read = adt7316_spi_read,
- .write = adt7316_spi_write,
- };
+ struct regmap *regmap;
/* don't exceed max specified SPI CLK frequency */
if (spi_dev->max_speed_hz > ADT7316_SPI_MAX_FREQ_HZ) {
@@ -87,12 +32,19 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
return -EINVAL;
}
+ regmap = devm_regmap_init_spi(spi_dev, _regmap_config);
+ if (IS_ERR(regmap)) {
+ dev_err(_dev->dev, "Error initializing spi regmap: %ld\n",
+ PTR_ERR(regmap));
+ return PTR_ERR(regmap);
+ }
+
/* switch from default I2C protocol to SPI protocol */
-
[PATCH v2 1/3] Staging: iio: adt7316: Remove irq from bus structure
interrupt request is not needed to be present in the bus
structure. It is a good option to pass it as a parameter
in the probe function instead of having it in the bus structure.
Signed-off-by: Shreeya Patel
---
drivers/staging/iio/addac/adt7316-i2c.c | 3 +--
drivers/staging/iio/addac/adt7316-spi.c | 4 ++--
drivers/staging/iio/addac/adt7316.c | 15 +++
drivers/staging/iio/addac/adt7316.h | 3 +--
4 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316-i2c.c
b/drivers/staging/iio/addac/adt7316-i2c.c
index ac91163656b5..335c17731cf7 100644
--- a/drivers/staging/iio/addac/adt7316-i2c.c
+++ b/drivers/staging/iio/addac/adt7316-i2c.c
@@ -103,14 +103,13 @@ static int adt7316_i2c_probe(struct i2c_client *client,
{
struct adt7316_bus bus = {
.client = client,
- .irq = client->irq,
.read = adt7316_i2c_read,
.write = adt7316_i2c_write,
.multi_read = adt7316_i2c_multi_read,
.multi_write = adt7316_i2c_multi_write,
};
- return adt7316_probe(>dev, , id->name);
+ return adt7316_probe(>dev, , id->name, client->irq);
}
static const struct i2c_device_id adt7316_i2c_id[] = {
diff --git a/drivers/staging/iio/addac/adt7316-spi.c
b/drivers/staging/iio/addac/adt7316-spi.c
index e75827e326a6..adaaa3eecd04 100644
--- a/drivers/staging/iio/addac/adt7316-spi.c
+++ b/drivers/staging/iio/addac/adt7316-spi.c
@@ -93,7 +93,6 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
{
struct adt7316_bus bus = {
.client = spi_dev,
- .irq = spi_dev->irq,
.read = adt7316_spi_read,
.write = adt7316_spi_write,
.multi_read = adt7316_spi_multi_read,
@@ -112,7 +111,8 @@ static int adt7316_spi_probe(struct spi_device *spi_dev)
adt7316_spi_write(spi_dev, 0, 0);
adt7316_spi_write(spi_dev, 0, 0);
- return adt7316_probe(_dev->dev, , spi_dev->modalias);
+ return adt7316_probe(_dev->dev, , spi_dev->modalias,
+spi_dev->irq);
}
static const struct spi_device_id adt7316_spi_id[] = {
diff --git a/drivers/staging/iio/addac/adt7316.c
b/drivers/staging/iio/addac/adt7316.c
index 1ca4ee0f30ee..6b4b80fd80cc 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -1807,12 +1807,12 @@ static irqreturn_t adt7316_event_handler(int irq, void
*private)
return IRQ_HANDLED;
}
-static int adt7316_setup_irq(struct iio_dev *indio_dev)
+static int adt7316_setup_irq(struct iio_dev *indio_dev, int irq)
{
struct adt7316_chip_info *chip = iio_priv(indio_dev);
int irq_type, ret;
- irq_type = irqd_get_trigger_type(irq_get_irq_data(chip->bus.irq));
+ irq_type = irqd_get_trigger_type(irq_get_irq_data(irq));
switch (irq_type) {
case IRQF_TRIGGER_HIGH:
@@ -1828,13 +1828,12 @@ static int adt7316_setup_irq(struct iio_dev *indio_dev)
break;
}
- ret = devm_request_threaded_irq(_dev->dev, chip->bus.irq,
+ ret = devm_request_threaded_irq(_dev->dev, irq,
NULL, adt7316_event_handler,
irq_type | IRQF_ONESHOT,
indio_dev->name, indio_dev);
if (ret) {
- dev_err(_dev->dev, "failed to request irq %d\n",
- chip->bus.irq);
+ dev_err(_dev->dev, "failed to request irq %d\n", irq);
return ret;
}
@@ -2134,7 +2133,7 @@ static const struct iio_info adt7516_info = {
* device probe and remove
*/
int adt7316_probe(struct device *dev, struct adt7316_bus *bus,
- const char *name)
+ const char *name, int irq)
{
struct adt7316_chip_info *chip;
struct iio_dev *indio_dev;
@@ -2180,8 +2179,8 @@ int adt7316_probe(struct device *dev, struct adt7316_bus
*bus,
indio_dev->name = name;
indio_dev->modes = INDIO_DIRECT_MODE;
- if (chip->bus.irq > 0) {
- ret = adt7316_setup_irq(indio_dev);
+ if (irq > 0) {
+ ret = adt7316_setup_irq(indio_dev, irq);
if (ret)
return ret;
}
diff --git a/drivers/staging/iio/addac/adt7316.h
b/drivers/staging/iio/addac/adt7316.h
index fd7c5c92b599..03d5300a98cd 100644
--- a/drivers/staging/iio/addac/adt7316.h
+++ b/drivers/staging/iio/addac/adt7316.h
@@ -16,7 +16,6 @@
struct adt7316_bus {
void *client;
- int irq;
int (*read)(void *client, u8 reg, u8 *data);
int (*write)(void *client, u8 reg, u8 val);
int (*multi_read)(void *client, u8 first_reg, u8 count, u8 *data);
@@ -30,6 +29,6 @@ extern const struct dev_pm_ops adt7316_pm_ops;
#define ADT7316_PM_OPS NULL
#endif
int adt7316_probe(struct device *dev, struct
[PATCH v2 0/3] adt7316 regmap implementation
This patchset consist of some initial patches for heading towards the regmap implementation and also the final patch which enables the driver to use regmap API thus removing the redundant and common code. Shreeya Patel (3): Staging: iio: adt7316: Remove irq from bus structure Staging: iio: adt7316: Remove multi read and write functions Staging: iio: adt7316: Add regmap support drivers/staging/iio/addac/adt7316-i2c.c | 101 ++-- drivers/staging/iio/addac/adt7316-spi.c | 95 +++ drivers/staging/iio/addac/adt7316.c | 147 drivers/staging/iio/addac/adt7316.h | 15 +-- 4 files changed, 103 insertions(+), 255 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On 1/19/19 2:25 AM, Christoph Hellwig wrote: On Fri, Jan 18, 2019 at 10:37:46AM -0800, Liam Mark wrote: Add support for configuring dma mapping attributes when mapping and unmapping memory through dma_buf_map_attachment and dma_buf_unmap_attachment. Signed-off-by: Liam Mark And who is going to decide which ones to pass? And who documents which ones are safe? I'd much rather have explicit, well documented dma-buf flags that might get translated to the DMA API flags, which are not error checked, not very well documented and way to easy to get wrong. I'm not sure having flags in dma-buf really solves anything given drivers can use the attributes directly with dma_map anyway, which is what we're looking to do. The intention is for the driver creating the dma_buf attachment to have the knowledge of which flags to use. Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: iio: ad7780: Add gain & filter gpio support
On Fri, 18 Jan 2019 18:19:40 -0200
Renato Lui Geh wrote:
> Hi,
>
> Sorry for the (extremely) late reply.
>
> Comments inline.
>
> Renato
>
> On 12/01, Jonathan Cameron wrote:
> >On Thu, 29 Nov 2018 11:02:46 -0200
> >Giuliano Augusto Faulin Belinassi wrote:
> >
> >> Hi
> >
> >A few follow ups from me having read the result in patch 2.
> >
> >Jonathan
> >>
> >> On Thu, Nov 29, 2018 at 9:18 AM Popa, Stefan Serban
> >> wrote:
> >> >
> >> > On Mi, 2018-11-28 at 16:15 -0200, Giuliano Belinassi wrote:
> >> > > Previously, the AD7780 driver only supported gpio for the 'powerdown'
> >> > > pin. This commit adds suppport for the 'gain' and 'filter' pin.
> >> > >
> >> > > Signed-off-by: Giuliano Belinassi
> >> > > ---
> >> > > Changes in v2:
> >> > > - Now this patch is part of the patchset that aims to remove ad7780
> >> > > out of staging. https://marc.info/?l=linux-iio=154282349808890=2
> >> > > - Also, now it reads voltage and filter values from the userspace
> >> > > instead of gpio pin states.
> >> >
> >> > Hello,
> >> > Please see bellow.
> >> >
> >> > >
> >> > > drivers/staging/iio/adc/ad7780.c | 78 --
> >> > > include/linux/iio/adc/ad_sigma_delta.h | 5 ++
> >> > > 2 files changed, 79 insertions(+), 4 deletions(-)
> >> > >
> >> > > diff --git a/drivers/staging/iio/adc/ad7780.c
> >> > > b/drivers/staging/iio/adc/ad7780.c
> >> > > index c4a85789c2db..05979a79fda3 100644
> >> > > --- a/drivers/staging/iio/adc/ad7780.c
> >> > > +++ b/drivers/staging/iio/adc/ad7780.c
> >> > > @@ -39,6 +39,12 @@
> >> > > #define AD7170_PATTERN (AD7780_PAT0 | AD7170_PAT2)
> >> > > #define AD7170_PATTERN_MASK (AD7780_PAT0 | AD7780_PAT1 |
> >> > > AD7170_PAT2)
> >> > >
> >> > > +#define AD7780_GAIN_GPIO 0
> >> > > +#define AD7780_FILTER_GPIO 1
> >> > > +
> >> > > +#define AD7780_GAIN_MIDPOINT 64
> >> > > +#define AD7780_FILTER_MIDPOINT 13350
> >> > > +
> >> > > struct ad7780_chip_info {
> >> > > struct iio_chan_specchannel;
> >> > > unsigned intpattern_mask;
> >> > > @@ -50,6 +56,8 @@ struct ad7780_state {
> >> > > const struct ad7780_chip_info *chip_info;
> >> > > struct regulator*reg;
> >> > > struct gpio_desc*powerdown_gpio;
> >> > > + struct gpio_desc*gain_gpio;
> >> > > + struct gpio_desc*filter_gpio;
> >> > > unsigned intgain;
> >> > >
> >> > > struct ad_sigma_delta sd;
> >> > > @@ -115,18 +123,65 @@ static int ad7780_read_raw(struct iio_dev
> >> > > *indio_dev,
> >> > > return -EINVAL;
> >> > > }
> >> > >
> >> > > +static int ad7780_write_raw(struct iio_dev *indio_dev,
> >> > > + struct iio_chan_spec const *chan,
> >> > > + int val,
> >> > > + int val2,
> >> > > + long m)
> >> > > +{
> >> > > + struct ad7780_state *st = iio_priv(indio_dev);
> >> > > + const struct ad7780_chip_info *chip_info = st->chip_info;
> >> > > + int uvref, gain;
> >> > > + unsigned int full_scale;
> >> > > +
> >> > > + if (!chip_info->is_ad778x)
> >> > > + return 0;
> >> > > +
> >> > > + switch (m) {
> >> > > + case IIO_CHAN_INFO_SCALE:
> >> > > + if (val != 0)
> >> > > + return -EINVAL;
> >> > > +
> >> > > + uvref = regulator_get_voltage(st->reg);
> >> >
> >> > regulator_get_voltage() has already been called in the probe function and
> >> > the result is stored in st->int_vref_mv.
> >>
> >> This was removed in commit 9eae69ddbc4717a0bd702eddac76c7848773bf71
> >> because the value was not being updated. But I agree if the vref
> >> voltage is not going to change at all after the initialization, then
> >> this value should be kept in memory.
>
> Why wouldn't the vref voltage not change after initialization? Shouldn't
> we keep reading and updating this in read_raw?
It may well change so bbest to check it.
> >>
> >> > My suggestion would be to use a local vref variable declared as unsigned
> >> > int. It is my fault that I haven't explained correctly in the previous
> >> > email, but you need to multiply vref_mv with 100LL in order to get
> >> > the
> >> > right precision: vref = st->int_vref_mv * 100LL. Afterwards you will
> >> > be
> >> > able to perform the divisions.
>
> Shouldn't we read vref inside read_raw in order to get up-to-date
> readings on voltage values? Or should we keep reading from a cached
> value?
I agree. We don't want to do it in a fast path as it 'probably' won't
change after the initial boot is done without some deliberate intervention
but fine to read it whenever we are dealing with anything other than
reading the ADC value (so reading the scale or similar).
> >>
> >> Thanks for this info! :-)
> >> Shouldn't we store this in uV (microVolts)? This will yield a more
> >> accurate result after
Re: [PATCH 0/5] binderfs: debug galore
On Fri, Jan 18, 2019 at 11:26:34PM +, Al Viro wrote:
> On Fri, Jan 18, 2019 at 03:53:39PM +0100, Christian Brauner wrote:
> > Hey everyone,
> >
> > Al gave me a really helpful review of binderfs and pointed out a range
> > of bugs. The most obvious and serious ones have fortunately already been
> > taken care of by patches sitting in Greg's char-misc-linus tree. The
> > others are hopefully all covered in this patchset.
>
> BTW, binderfs_binder_device_create() looks rather odd - it would be easier
> to do this:
> inode_lock(d_inode(root));
> /* look it up */
> dentry = lookup_one_len(name, root, strlen(name));
It didn't seem obvious that that's what you should use to lookup
dentries instead of d_lookup(). Especially since d_lookup() points out
that it takes the rename lock which seemed crucial to me so that we
don't end up in a situation where we race against another dentry being
renamed to the name we're currently trying to used.
Thanks for the pointer!
> if (IS_ERR(dentry)) {
> /* some kind of error (ENOMEM, permissions) - report */
> inode_unlock(d_inode(root));
> ret = PTR_ERR(dentry);
> goto err;
> }
> if (d_really_is_positive(dentry)) {
> /* already exists */
> dput(dentry);
> inode_unlock(d_inode(root));
> ret = -EEXIST;
> goto err;
> }
> inode->i_private = device;
> ... and from that point on - as in your variant. Another thing in there:
Right, just read through the code for lookup_one_len() it seems to
allocate a new dentry if it can't find a hashed match for the old name.
That's surprising. The name of the function does not really give that
impression. :) (Would probably be better if lookup_or_alloc_one_len() or
something. But that's not important now.)
> name = kmalloc(name_len, GFP_KERNEL);
> if (!name)
> goto err;
>
> strscpy(name, req->name, name_len);
> is an odd way to go; more straightforward would be
> req->name[BINDERFS_MAX_NAME] = '\0';/* NUL-terminate */
> name = kmemdup(req->name, sizeof(req->name), GEP_KERNEL);
> if (!name)
>
Using kmemdup() now.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] binderfs: rework binderfs_fill_super()
On Fri, Jan 18, 2019 at 11:03:54PM +, Al Viro wrote:
> On Fri, Jan 18, 2019 at 03:53:42PM +0100, Christian Brauner wrote:
> > static int binderfs_fill_super(struct super_block *sb, void *data, int
> > silent)
> > {
> > + int ret;
> > struct binderfs_info *info;
> > - int ret = -ENOMEM;
> > struct inode *inode = NULL;
> > struct ipc_namespace *ipc_ns = current->nsproxy->ipc_ns;
> >
> > @@ -495,13 +495,14 @@ static int binderfs_fill_super(struct super_block
> > *sb, void *data, int silent)
> > sb->s_op = _super_ops;
> > sb->s_time_gran = 1;
> >
> > - info = kzalloc(sizeof(struct binderfs_info), GFP_KERNEL);
> > - if (!info)
> > - goto err_without_dentry;
> > + sb->s_fs_info = kzalloc(sizeof(struct binderfs_info), GFP_KERNEL);
> > + if (!sb->s_fs_info)
> > + return -ENOMEM;
> > + info = sb->s_fs_info;
>
> ... and that's when you should grab ipcns reference and stick it into
> info->ipc_ns, to match the logics in binderfs_kill_super().
>
> Otherwise the failure above
>
> > ret = binderfs_parse_mount_opts(data, >mount_opts);
> > if (ret)
> > - goto err_without_dentry;
> > + return ret;
>
> ... or here leaves you with an ipcns leak.
>
> Destructor does
> if ->s_fs_info is non-NULL
> release ->s_fs_info->ipc_ns
> free ->s_fs_info
> so constructor should not leave object in a state when ipcns is already
> grabbed, but not stored in ->s_fs_info->ipc_ns (including the case of
> allocation failure leaving it with NULL ->s_fs_info).
Yeah, total brainfart on my side. I shouldn't code in airports
apparently... Fixed.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/5] binderfs: prevent renaming the control dentry
On Fri, Jan 18, 2019 at 10:55:52PM +, Al Viro wrote:
> On Fri, Jan 18, 2019 at 03:53:41PM +0100, Christian Brauner wrote:
> > We don't allow to unlink it since it is crucial for binderfs to be useable
> > but if we allow to rename it we make the unlink trivial to bypass. So
> > prevent renaming too and simply treat the control dentry as immutable.
> >
> > Take the opportunity and turn the check for the control dentry into a
> > separate helper is_binderfs_control_device() since it's now used in two
> > places.
> > Additionally, replace the custom rename dance we did with call to
> > simple_rename().
>
> Umm...
>
> > +static inline bool is_binderfs_control_device(const struct inode *inode,
> > + const struct dentry *dentry)
> > +{
> > + return BINDERFS_I(inode)->control_dentry == dentry;
> > +}
>
> What do you need an inode for?
BINDERFS_I() is called in is_binderfs_device() which is called in
binder.c:
static int binder_open(struct inode *nodp, struct file *filp)
{
/* binderfs stashes devices in i_private */
if (is_binderfs_device(nodp))
binder_dev = nodp->i_private;
else
binder_dev = container_of(filp->private_data,
struct binder_device, miscdev);
and it was just easier to have it take an inode as arg since that's what
binder_open() makes easily available. Just kept it this way for
is_binderfs_control_device() too but will switch the latter over now.
>
> static inline struct binderfs_info *BINDERFS_I(const struct inode *inode)
> {
> return inode->i_sb->s_fs_info;
> }
>
> so it looks like all you care about is the superblock. Which can be
> had simply as dentry->d_sb...
Hm yes, I think I'll just do:
static inline bool is_binderfs_control_device(const struct dentry *dentry)
{
struct binderfs_info *info = dentry->d_sb->s_fs_info;
return info->control_dentry == dentry;
}
and pick up what you scratched below.
>
> Besides, what's the point of calling is_binderfs_device() in ->rename()?
> If your directory methods are given dentries from another filesystem,
> the kernel is already FUBAR. So your rename should simply do
> if (is_binderfs_control_device(old_dentry) ||
> is_binderfs_control_device(new_dentry))
> return -EPERM;
> return simple_rename(..);
> and that's it...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] sched/wait: introduce wait_event_freezable_hrtimeout
> > as far as I understand this code, freezable_schedule() avoids blocking the > > freezer during the schedule() call, but in the end try_to_freeze() is still > > called so the result is the same, right? > > I wonder why wait_event_freezable is not calling freezable_schedule(). > > It could be something subtle in my view. freezable_schedule() actually makes > the freezer code not send a wake up to the sleeping task if a freeze happens, > because the PF_FREEZER_SKIP flag is set, as you pointed. > > Whereas wait_event_freezable() which uses try_to_freeze() does not seem to > have > this behavior and the task will enter the freezer. So I'm a bit skeptical if > your API will behave as expected (or at least consistently with other wait > APIs). oh right, now it is clear to me: - schedule(); try_to_freeze() schedule() is called and the task enters sleep. Since PF_FREEZER_SKIP is not set, the task wakes up as soon as try_to_freeze_tasks() is called. Right after waking up the task calls try_to_freeze() which freezes it. - freezable_schedule() schedule() is called and the task enters sleep. Since PF_FREEZER_SKIP is set, the task does not wake up when try_to_freeze_tasks() is called. This is not a problem, the task can't "do anything which isn't allowed for a frozen task" while sleeping[0]. When the task wakes up (timeout, or whatever other reason) it calls try_to_freeze() which freezes it if the freeze is still underway. So if a freeze is triggered while the task is sleeping, a task executing freezable_schedule() might or might not notice the freeze depending on how long it sleeps. A task executing schedule(); try_to_freeze() will always notice it. I might be wrong on that, but freezable_schedule() just seems like a performance improvement to me. Now I fully agree with you that there should be a uniform definition of "freezable" between wait_event_freezable and wait_event_freezable_hrtimeout. This leaves me to the question: should I modify my definition of wait_event_freezable_hrtimeout, or prepare a patch for wait_event_freezable ? If I am right with the performance thing, the latter might be worth considering? Either way, this will be fixed in the v2. > > That being said, I am not sure that the try_to_freeze() call does anything > > in the vsoc case because there is no call to set_freezable() so the thread > > still has PF_NOFREEZE... > > I traced this, and PF_NOFREEZE is not set by default for tasks. Well, I did not check this in practice and might be confused somewhere but the documentation[1] says "kernel threads are not freezable by default. However, a kernel thread may clear PF_NOFREEZE for itself by calling set_freezable()". Looking at the kthreadd() definition it seems like new tasks have PF_NOFREEZE set by default[2]. I'll take some time to check this in practice in the next days. Anyways, thanks for your time ! regards, Hugo [0] https://elixir.bootlin.com/linux/latest/source/include/linux/freezer.h#L103 [1] https://elixir.bootlin.com/linux/latest/source/Documentation/power/freezing-of-tasks.txt#L90 [2] https://elixir.bootlin.com/linux/latest/source/kernel/kthread.c#L569 -- Hugo Lefeuvre (hle)|www.owl.eu.com RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On Fri, Jan 18, 2019 at 10:37:46AM -0800, Liam Mark wrote: > Add support for configuring dma mapping attributes when mapping > and unmapping memory through dma_buf_map_attachment and > dma_buf_unmap_attachment. > > Signed-off-by: Liam Mark And who is going to decide which ones to pass? And who documents which ones are safe? I'd much rather have explicit, well documented dma-buf flags that might get translated to the DMA API flags, which are not error checked, not very well documented and way to easy to get wrong. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap
On Fri, Jan 18, 2019 at 12:31:34PM -0800, Laura Abbott wrote: > I thought about doing that, the problem is it becomes an ABI break for > existing users which I really didn't want to do again. If it > ends up being the last thing we do before moving out of staging, > I'd consider doing it. This is staging code, so any existing users by defintion do not matter. Let's not drag Linux development down over this. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Just edit it
Want to start editing your photos? We are studio special for photo shooting and retouching. For editing your photos: We can do white background, sharpen, retouching included Unlimited revisions accepted You can give us a test photo, we will do it for you. Thanks, Maggie ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
