2019-07-03 14:57:27

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 0/3] soc: fsl: dpio: small fixes

This patch set contains a couple of fixes that concern the dpio driver and
the fsl-mc bus. The first two patches are adapting the usage of device
links to match newer developments in the area. The third patch fixes the
Kconfig of FSL_MC_DPIO by selecting directly FSL_GUTS instead of SOC_BUS.

Ioana Ciornei (3):
bus: fsl-mc: remove explicit device_link_del
soc: fsl: dpio: remove explicit device_link_remove
soc: fsl: FSL_MC_DPIO selects directly FSL_GUTS

drivers/bus/fsl-mc/fsl-mc-allocator.c | 1 -
drivers/bus/fsl-mc/mc-io.c | 1 -
drivers/soc/fsl/Kconfig | 2 +-
drivers/soc/fsl/dpio/dpio-service.c | 2 --
4 files changed, 1 insertion(+), 5 deletions(-)

--
1.9.1


2019-07-03 14:57:42

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 2/3] soc: fsl: dpio: remove explicit device_link_remove

Starting with commit 72175d4ea4c4 ("driver core: Make driver core
own stateful device links") stateful device links are owned by the
driver core and should not be explicitly removed on device unbind.
Delete all device_link_remove appearances from the dpio driver.

Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/soc/fsl/dpio/dpio-service.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
index b9539ef2c3cd..518a8e081b49 100644
--- a/drivers/soc/fsl/dpio/dpio-service.c
+++ b/drivers/soc/fsl/dpio/dpio-service.c
@@ -305,8 +305,6 @@ void dpaa2_io_service_deregister(struct dpaa2_io *service,
list_del(&ctx->node);
spin_unlock_irqrestore(&d->lock_notifications, irqflags);

- if (dev)
- device_link_remove(dev, d->dev);
}
EXPORT_SYMBOL_GPL(dpaa2_io_service_deregister);

--
1.9.1

2019-07-03 14:57:50

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 3/3] soc: fsl: FSL_MC_DPIO selects directly FSL_GUTS

Make FSL_MC_DPIO select directly FSL_GUTS. Without this change we could
be in a situation where both FSL_MC_DPIO and SOC_BUS are enabled but
FSL_GUTS is not.

Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/soc/fsl/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
index b6804c04e96f..7e62c1d0aee7 100644
--- a/drivers/soc/fsl/Kconfig
+++ b/drivers/soc/fsl/Kconfig
@@ -22,7 +22,7 @@ config FSL_GUTS
config FSL_MC_DPIO
tristate "QorIQ DPAA2 DPIO driver"
depends on FSL_MC_BUS
- select SOC_BUS
+ select FSL_GUTS
help
Driver for the DPAA2 DPIO object. A DPIO provides queue and
buffer management facilities for software to interact with
--
1.9.1

2019-07-03 14:59:28

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 1/3] bus: fsl-mc: remove explicit device_link_del

Starting with commit 72175d4ea4c4 ("driver core: Make driver core own
stateful device links") stateful device links are owned by the driver
core and should not be explicitly removed on device unbind. Delete all
device_link_del appearances from the fsl-mc bus.

Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/bus/fsl-mc/fsl-mc-allocator.c | 1 -
drivers/bus/fsl-mc/mc-io.c | 1 -
2 files changed, 2 deletions(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-allocator.c b/drivers/bus/fsl-mc/fsl-mc-allocator.c
index 8ad77246f322..cc7bb900f524 100644
--- a/drivers/bus/fsl-mc/fsl-mc-allocator.c
+++ b/drivers/bus/fsl-mc/fsl-mc-allocator.c
@@ -330,7 +330,6 @@ void fsl_mc_object_free(struct fsl_mc_device *mc_adev)

fsl_mc_resource_free(resource);

- device_link_del(mc_adev->consumer_link);
mc_adev->consumer_link = NULL;
}
EXPORT_SYMBOL_GPL(fsl_mc_object_free);
diff --git a/drivers/bus/fsl-mc/mc-io.c b/drivers/bus/fsl-mc/mc-io.c
index 3ae574a58cce..d9629fc13a15 100644
--- a/drivers/bus/fsl-mc/mc-io.c
+++ b/drivers/bus/fsl-mc/mc-io.c
@@ -255,7 +255,6 @@ void fsl_mc_portal_free(struct fsl_mc_io *mc_io)
fsl_destroy_mc_io(mc_io);
fsl_mc_resource_free(resource);

- device_link_del(dpmcp_dev->consumer_link);
dpmcp_dev->consumer_link = NULL;
}
EXPORT_SYMBOL_GPL(fsl_mc_portal_free);
--
1.9.1

2019-08-15 23:55:09

by Leo Li

[permalink] [raw]
Subject: Re: [PATCH 3/3] soc: fsl: FSL_MC_DPIO selects directly FSL_GUTS

On Wed, Jul 3, 2019 at 9:58 AM Ioana Ciornei <[email protected]> wrote:
>
> Make FSL_MC_DPIO select directly FSL_GUTS. Without this change we could
> be in a situation where both FSL_MC_DPIO and SOC_BUS are enabled but
> FSL_GUTS is not.
>
> Signed-off-by: Ioana Ciornei <[email protected]>
> ---
> drivers/soc/fsl/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> index b6804c04e96f..7e62c1d0aee7 100644
> --- a/drivers/soc/fsl/Kconfig
> +++ b/drivers/soc/fsl/Kconfig
> @@ -22,7 +22,7 @@ config FSL_GUTS
> config FSL_MC_DPIO
> tristate "QorIQ DPAA2 DPIO driver"
> depends on FSL_MC_BUS
> - select SOC_BUS
> + select FSL_GUTS

NACK. Although DPIO only exists on SoCs with the GUTS block for now.
There is no direct dependency between the two IPs. I don't think we
should add this dependency to make FSL_GUTS not configurable. Here is
some explaination from kernel documentation:

select should be used with care. select will force
a symbol to a value without visiting the dependencies.
By abusing select you are able to select a symbol FOO even
if FOO depends on BAR that is not set.
In general use select only for non-visible symbols
(no prompts anywhere) and for symbols with no dependencies.
That will limit the usefulness but on the other hand avoid
the illegal configurations all over.

We probably shouldn't let it select SOC_BUS either from the beginning,
as the basic feature of DPIO should still work without defining
SOC_BUS.

Regards,
Leo

> help
> Driver for the DPAA2 DPIO object. A DPIO provides queue and
> buffer management facilities for software to interact with
> --
> 1.9.1
>

2019-08-15 23:55:33

by Leo Li

[permalink] [raw]
Subject: Re: [PATCH 1/3] bus: fsl-mc: remove explicit device_link_del

On Wed, Jul 3, 2019 at 9:58 AM Ioana Ciornei <[email protected]> wrote:
>
> Starting with commit 72175d4ea4c4 ("driver core: Make driver core own
> stateful device links") stateful device links are owned by the driver
> core and should not be explicitly removed on device unbind. Delete all
> device_link_del appearances from the fsl-mc bus.
>
> Signed-off-by: Ioana Ciornei <[email protected]>

Hi Laurentiu,

What do you think of this patches? I can take it through fsl/soc if
you can ACK it.

Regards,
Leo

> ---
> drivers/bus/fsl-mc/fsl-mc-allocator.c | 1 -
> drivers/bus/fsl-mc/mc-io.c | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-allocator.c b/drivers/bus/fsl-mc/fsl-mc-allocator.c
> index 8ad77246f322..cc7bb900f524 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-allocator.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-allocator.c
> @@ -330,7 +330,6 @@ void fsl_mc_object_free(struct fsl_mc_device *mc_adev)
>
> fsl_mc_resource_free(resource);
>
> - device_link_del(mc_adev->consumer_link);
> mc_adev->consumer_link = NULL;
> }
> EXPORT_SYMBOL_GPL(fsl_mc_object_free);
> diff --git a/drivers/bus/fsl-mc/mc-io.c b/drivers/bus/fsl-mc/mc-io.c
> index 3ae574a58cce..d9629fc13a15 100644
> --- a/drivers/bus/fsl-mc/mc-io.c
> +++ b/drivers/bus/fsl-mc/mc-io.c
> @@ -255,7 +255,6 @@ void fsl_mc_portal_free(struct fsl_mc_io *mc_io)
> fsl_destroy_mc_io(mc_io);
> fsl_mc_resource_free(resource);
>
> - device_link_del(dpmcp_dev->consumer_link);
> dpmcp_dev->consumer_link = NULL;
> }
> EXPORT_SYMBOL_GPL(fsl_mc_portal_free);
> --
> 1.9.1
>

2019-08-15 23:56:17

by Leo Li

[permalink] [raw]
Subject: Re: [PATCH 2/3] soc: fsl: dpio: remove explicit device_link_remove

On Wed, Jul 3, 2019 at 9:58 AM Ioana Ciornei <[email protected]> wrote:
>
> Starting with commit 72175d4ea4c4 ("driver core: Make driver core
> own stateful device links") stateful device links are owned by the
> driver core and should not be explicitly removed on device unbind.
> Delete all device_link_remove appearances from the dpio driver.
>
> Signed-off-by: Ioana Ciornei <[email protected]>

Applied for next. Thanks.

> ---
> drivers/soc/fsl/dpio/dpio-service.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/dpio-service.c b/drivers/soc/fsl/dpio/dpio-service.c
> index b9539ef2c3cd..518a8e081b49 100644
> --- a/drivers/soc/fsl/dpio/dpio-service.c
> +++ b/drivers/soc/fsl/dpio/dpio-service.c
> @@ -305,8 +305,6 @@ void dpaa2_io_service_deregister(struct dpaa2_io *service,
> list_del(&ctx->node);
> spin_unlock_irqrestore(&d->lock_notifications, irqflags);
>
> - if (dev)
> - device_link_remove(dev, d->dev);
> }
> EXPORT_SYMBOL_GPL(dpaa2_io_service_deregister);
>
> --
> 1.9.1
>

2019-08-16 06:03:55

by Laurentiu Tudor

[permalink] [raw]
Subject: RE: [PATCH 1/3] bus: fsl-mc: remove explicit device_link_del

Hi Leo,

> -----Original Message-----
> From: Li Yang <[email protected]>
> Sent: Friday, August 16, 2019 2:13 AM
> To: Ioana Ciornei <[email protected]>
> Cc: Laurentiu Tudor <[email protected]>; Roy Pledge
> <[email protected]>; lkml <[email protected]>
> Subject: Re: [PATCH 1/3] bus: fsl-mc: remove explicit device_link_del
> Importance: High
>
> On Wed, Jul 3, 2019 at 9:58 AM Ioana Ciornei <[email protected]> wrote:
> >
> > Starting with commit 72175d4ea4c4 ("driver core: Make driver core own
> > stateful device links") stateful device links are owned by the driver
> > core and should not be explicitly removed on device unbind. Delete all
> > device_link_del appearances from the fsl-mc bus.
> >
> > Signed-off-by: Ioana Ciornei <[email protected]>
>
> Hi Laurentiu,
>
> What do you think of this patches? I can take it through fsl/soc if
> you can ACK it.

Looks good to me, so for the whole series:

Acked-By: Laurentiu Tudor <[email protected]>

> Regards,
> Leo
>
> > ---
> > drivers/bus/fsl-mc/fsl-mc-allocator.c | 1 -
> > drivers/bus/fsl-mc/mc-io.c | 1 -
> > 2 files changed, 2 deletions(-)
> >
> > diff --git a/drivers/bus/fsl-mc/fsl-mc-allocator.c b/drivers/bus/fsl-
> mc/fsl-mc-allocator.c
> > index 8ad77246f322..cc7bb900f524 100644
> > --- a/drivers/bus/fsl-mc/fsl-mc-allocator.c
> > +++ b/drivers/bus/fsl-mc/fsl-mc-allocator.c
> > @@ -330,7 +330,6 @@ void fsl_mc_object_free(struct fsl_mc_device
> *mc_adev)
> >
> > fsl_mc_resource_free(resource);
> >
> > - device_link_del(mc_adev->consumer_link);
> > mc_adev->consumer_link = NULL;
> > }
> > EXPORT_SYMBOL_GPL(fsl_mc_object_free);
> > diff --git a/drivers/bus/fsl-mc/mc-io.c b/drivers/bus/fsl-mc/mc-io.c
> > index 3ae574a58cce..d9629fc13a15 100644
> > --- a/drivers/bus/fsl-mc/mc-io.c
> > +++ b/drivers/bus/fsl-mc/mc-io.c
> > @@ -255,7 +255,6 @@ void fsl_mc_portal_free(struct fsl_mc_io *mc_io)
> > fsl_destroy_mc_io(mc_io);
> > fsl_mc_resource_free(resource);
> >
> > - device_link_del(dpmcp_dev->consumer_link);
> > dpmcp_dev->consumer_link = NULL;
> > }
> > EXPORT_SYMBOL_GPL(fsl_mc_portal_free);
> > --
> > 1.9.1
> >

2019-08-16 14:59:39

by Roy Pledge

[permalink] [raw]
Subject: Re: [PATCH 3/3] soc: fsl: FSL_MC_DPIO selects directly FSL_GUTS

On 8/15/2019 7:09 PM, Li Yang wrote:
> On Wed, Jul 3, 2019 at 9:58 AM Ioana Ciornei <[email protected]> wrote:
>> Make FSL_MC_DPIO select directly FSL_GUTS. Without this change we could
>> be in a situation where both FSL_MC_DPIO and SOC_BUS are enabled but
>> FSL_GUTS is not.
>>
>> Signed-off-by: Ioana Ciornei <[email protected]>
>> ---
>> drivers/soc/fsl/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
>> index b6804c04e96f..7e62c1d0aee7 100644
>> --- a/drivers/soc/fsl/Kconfig
>> +++ b/drivers/soc/fsl/Kconfig
>> @@ -22,7 +22,7 @@ config FSL_GUTS
>> config FSL_MC_DPIO
>> tristate "QorIQ DPAA2 DPIO driver"
>> depends on FSL_MC_BUS
>> - select SOC_BUS
>> + select FSL_GUTS
> NACK. Although DPIO only exists on SoCs with the GUTS block for now.
> There is no direct dependency between the two IPs. I don't think we
There isn't a hardware dependency but the DPIO driver does call
soc_device_match(). If FSL_GUTS isn't present we can't distinguish
which SoC is being used and the driver isn't able to setup the correct
stashing destinations. Is there another mechanism we should be using to
get this information?
> should add this dependency to make FSL_GUTS not configurable. Here is
> some explaination from kernel documentation:
>
> select should be used with care. select will force
> a symbol to a value without visiting the dependencies.
> By abusing select you are able to select a symbol FOO even
> if FOO depends on BAR that is not set.
> In general use select only for non-visible symbols
> (no prompts anywhere) and for symbols with no dependencies.
> That will limit the usefulness but on the other hand avoid
> the illegal configurations all over.
>
> We probably shouldn't let it select SOC_BUS either from the beginning,
> as the basic feature of DPIO should still work without defining
> SOC_BUS.
>
> Regards,
> Leo
>
>> help
>> Driver for the DPAA2 DPIO object. A DPIO provides queue and
>> buffer management facilities for software to interact with
>> --
>> 1.9.1
>>

2019-08-16 17:04:58

by Leo Li

[permalink] [raw]
Subject: Re: [PATCH 3/3] soc: fsl: FSL_MC_DPIO selects directly FSL_GUTS

On Fri, Aug 16, 2019 at 9:59 AM Roy Pledge <[email protected]> wrote:
>
> On 8/15/2019 7:09 PM, Li Yang wrote:
> > On Wed, Jul 3, 2019 at 9:58 AM Ioana Ciornei <[email protected]> wrote:
> >> Make FSL_MC_DPIO select directly FSL_GUTS. Without this change we could
> >> be in a situation where both FSL_MC_DPIO and SOC_BUS are enabled but
> >> FSL_GUTS is not.
> >>
> >> Signed-off-by: Ioana Ciornei <[email protected]>
> >> ---
> >> drivers/soc/fsl/Kconfig | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> >> index b6804c04e96f..7e62c1d0aee7 100644
> >> --- a/drivers/soc/fsl/Kconfig
> >> +++ b/drivers/soc/fsl/Kconfig
> >> @@ -22,7 +22,7 @@ config FSL_GUTS
> >> config FSL_MC_DPIO
> >> tristate "QorIQ DPAA2 DPIO driver"
> >> depends on FSL_MC_BUS
> >> - select SOC_BUS
> >> + select FSL_GUTS
> > NACK. Although DPIO only exists on SoCs with the GUTS block for now.
> > There is no direct dependency between the two IPs. I don't think we
> There isn't a hardware dependency but the DPIO driver does call
> soc_device_match(). If FSL_GUTS isn't present we can't distinguish
> which SoC is being used and the driver isn't able to setup the correct
> stashing destinations. Is there another mechanism we should be using to
> get this information?

The build will still succeed if SOC_BUS is not defined. The only
thing is you cannot get the SoC information. If the driver is really
depending on the SoC information, it probably should "depends on
SOC_BUS" instead of "select SOC_BUS". My understanding is that the
stashing is an optional performance optimization of DPIO and DPIO can
still be working with out it, right? If that is the case, we probably
should do nothing here. Or create another stashing related option to
"depends on SOC_BUS".

Regards,
Leo

> > should add this dependency to make FSL_GUTS not configurable. Here is
> > some explaination from kernel documentation:
> >
> > select should be used with care. select will force
> > a symbol to a value without visiting the dependencies.
> > By abusing select you are able to select a symbol FOO even
> > if FOO depends on BAR that is not set.
> > In general use select only for non-visible symbols
> > (no prompts anywhere) and for symbols with no dependencies.
> > That will limit the usefulness but on the other hand avoid
> > the illegal configurations all over.
> >
> > We probably shouldn't let it select SOC_BUS either from the beginning,
> > as the basic feature of DPIO should still work without defining
> > SOC_BUS.
> >
> > Regards,
> > Leo
> >
> >> help
> >> Driver for the DPAA2 DPIO object. A DPIO provides queue and
> >> buffer management facilities for software to interact with
> >> --
> >> 1.9.1
> >>
>

2019-08-16 19:55:32

by Leo Li

[permalink] [raw]
Subject: Re: [PATCH 1/3] bus: fsl-mc: remove explicit device_link_del

On Fri, Aug 16, 2019 at 1:04 AM Laurentiu Tudor <[email protected]> wrote:
>
> Hi Leo,
>
> > -----Original Message-----
> > From: Li Yang <[email protected]>
> > Sent: Friday, August 16, 2019 2:13 AM
> > To: Ioana Ciornei <[email protected]>
> > Cc: Laurentiu Tudor <[email protected]>; Roy Pledge
> > <[email protected]>; lkml <[email protected]>
> > Subject: Re: [PATCH 1/3] bus: fsl-mc: remove explicit device_link_del
> > Importance: High
> >
> > On Wed, Jul 3, 2019 at 9:58 AM Ioana Ciornei <[email protected]> wrote:
> > >
> > > Starting with commit 72175d4ea4c4 ("driver core: Make driver core own
> > > stateful device links") stateful device links are owned by the driver
> > > core and should not be explicitly removed on device unbind. Delete all
> > > device_link_del appearances from the fsl-mc bus.
> > >
> > > Signed-off-by: Ioana Ciornei <[email protected]>
> >
> > Hi Laurentiu,
> >
> > What do you think of this patches? I can take it through fsl/soc if
> > you can ACK it.
>
> Looks good to me, so for the whole series:
>
> Acked-By: Laurentiu Tudor <[email protected]>

Thanks. Applied for next.


>
> > Regards,
> > Leo
> >
> > > ---
> > > drivers/bus/fsl-mc/fsl-mc-allocator.c | 1 -
> > > drivers/bus/fsl-mc/mc-io.c | 1 -
> > > 2 files changed, 2 deletions(-)
> > >
> > > diff --git a/drivers/bus/fsl-mc/fsl-mc-allocator.c b/drivers/bus/fsl-
> > mc/fsl-mc-allocator.c
> > > index 8ad77246f322..cc7bb900f524 100644
> > > --- a/drivers/bus/fsl-mc/fsl-mc-allocator.c
> > > +++ b/drivers/bus/fsl-mc/fsl-mc-allocator.c
> > > @@ -330,7 +330,6 @@ void fsl_mc_object_free(struct fsl_mc_device
> > *mc_adev)
> > >
> > > fsl_mc_resource_free(resource);
> > >
> > > - device_link_del(mc_adev->consumer_link);
> > > mc_adev->consumer_link = NULL;
> > > }
> > > EXPORT_SYMBOL_GPL(fsl_mc_object_free);
> > > diff --git a/drivers/bus/fsl-mc/mc-io.c b/drivers/bus/fsl-mc/mc-io.c
> > > index 3ae574a58cce..d9629fc13a15 100644
> > > --- a/drivers/bus/fsl-mc/mc-io.c
> > > +++ b/drivers/bus/fsl-mc/mc-io.c
> > > @@ -255,7 +255,6 @@ void fsl_mc_portal_free(struct fsl_mc_io *mc_io)
> > > fsl_destroy_mc_io(mc_io);
> > > fsl_mc_resource_free(resource);
> > >
> > > - device_link_del(dpmcp_dev->consumer_link);
> > > dpmcp_dev->consumer_link = NULL;
> > > }
> > > EXPORT_SYMBOL_GPL(fsl_mc_portal_free);
> > > --
> > > 1.9.1
> > >