Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

2022-07-01 Thread Tony Lindgren
* Saravana Kannan  [220701 08:21]:
> On Fri, Jul 1, 2022 at 1:10 AM Saravana Kannan  wrote:
> >
> > On Thu, Jun 30, 2022 at 11:12 PM Tony Lindgren  wrote:
> > >
> > > * Tony Lindgren  [220701 08:33]:
> > > > * Saravana Kannan  [220630 23:25]:
> > > > > On Thu, Jun 30, 2022 at 4:26 PM Rob Herring  wrote:
> > > > > >
> > > > > > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan 
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > * Saravana Kannan  [220623 08:17]:
> > > > > > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > * Saravana Kannan  [220622 19:05]:
> > > > > > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > > This issue is no directly related fw_devlink. It is a 
> > > > > > > > > > > > side effect of
> > > > > > > > > > > > removing driver_deferred_probe_check_state(). We no 
> > > > > > > > > > > > longer return
> > > > > > > > > > > > -EPROBE_DEFER at the end of 
> > > > > > > > > > > > driver_deferred_probe_check_state().
> > > > > > > > > > >
> > > > > > > > > > > Yes, I understand the issue. But 
> > > > > > > > > > > driver_deferred_probe_check_state()
> > > > > > > > > > > was deleted because fw_devlink=on should have short 
> > > > > > > > > > > circuited the
> > > > > > > > > > > probe attempt with an  -EPROBE_DEFER before reaching the 
> > > > > > > > > > > bus/driver
> > > > > > > > > > > probe function and hitting this -ENOENT failure. That's 
> > > > > > > > > > > why I was
> > > > > > > > > > > asking the other questions.
> > > > > > > > > >
> > > > > > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > > > > > > driver_deferred_probe_check_state() then?
> > > > > > > > >
> > > > > > > > > device_links_check_suppliers() call inside really_probe() 
> > > > > > > > > would short
> > > > > > > > > circuit and return an -EPROBE_DEFER if the device links are 
> > > > > > > > > created as
> > > > > > > > > expected.
> > > > > > > >
> > > > > > > > OK
> > > > > > > >
> > > > > > > > > > Hmm so I'm not seeing any supplier for the top level ocp 
> > > > > > > > > > device in
> > > > > > > > > > the booting case without your patches. I see the suppliers 
> > > > > > > > > > for the
> > > > > > > > > > ocp child device instances only.
> > > > > > > > >
> > > > > > > > > Hmmm... this is strange (that the device link isn't there), 
> > > > > > > > > but this
> > > > > > > > > is what I suspected.
> > > > > > > >
> > > > > > > > Yup, maybe it's because of the supplier being a device in the 
> > > > > > > > child
> > > > > > > > interconnect for the ocp.
> > > > > > >
> > > > > > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device 
> > > > > > > link
> > > > > > > isn't being created.
> > > > > > >
> > > > > > > So the aggregated view is something like (I had to set tabs = 4 
> > > > > > > space
> > > > > > > to fit it within 80 cols):
> > > > > > >
> > > > > > > ocp: ocp { <

Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

2022-06-30 Thread Tony Lindgren
* Tony Lindgren  [220701 08:33]:
> * Saravana Kannan  [220630 23:25]:
> > On Thu, Jun 30, 2022 at 4:26 PM Rob Herring  wrote:
> > >
> > > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan  
> > > wrote:
> > > >
> > > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren  wrote:
> > > > >
> > > > > * Saravana Kannan  [220623 08:17]:
> > > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren  
> > > > > > wrote:
> > > > > > >
> > > > > > > * Saravana Kannan  [220622 19:05]:
> > > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren 
> > > > > > > >  wrote:
> > > > > > > > > This issue is no directly related fw_devlink. It is a side 
> > > > > > > > > effect of
> > > > > > > > > removing driver_deferred_probe_check_state(). We no longer 
> > > > > > > > > return
> > > > > > > > > -EPROBE_DEFER at the end of 
> > > > > > > > > driver_deferred_probe_check_state().
> > > > > > > >
> > > > > > > > Yes, I understand the issue. But 
> > > > > > > > driver_deferred_probe_check_state()
> > > > > > > > was deleted because fw_devlink=on should have short circuited 
> > > > > > > > the
> > > > > > > > probe attempt with an  -EPROBE_DEFER before reaching the 
> > > > > > > > bus/driver
> > > > > > > > probe function and hitting this -ENOENT failure. That's why I 
> > > > > > > > was
> > > > > > > > asking the other questions.
> > > > > > >
> > > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > > > driver_deferred_probe_check_state() then?
> > > > > >
> > > > > > device_links_check_suppliers() call inside really_probe() would 
> > > > > > short
> > > > > > circuit and return an -EPROBE_DEFER if the device links are created 
> > > > > > as
> > > > > > expected.
> > > > >
> > > > > OK
> > > > >
> > > > > > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > > > > > the booting case without your patches. I see the suppliers for the
> > > > > > > ocp child device instances only.
> > > > > >
> > > > > > Hmmm... this is strange (that the device link isn't there), but this
> > > > > > is what I suspected.
> > > > >
> > > > > Yup, maybe it's because of the supplier being a device in the child
> > > > > interconnect for the ocp.
> > > >
> > > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
> > > > isn't being created.
> > > >
> > > > So the aggregated view is something like (I had to set tabs = 4 space
> > > > to fit it within 80 cols):
> > > >
> > > > ocp: ocp { <= Consumer
> > > > compatible = "simple-pm-bus";
> > > > power-domains = <&prm_per>; <=== Supplier ref
> > > >
> > > > l4_wkup: interconnect@44c0 {
> > > > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
> > > >
> > > > segment@20 {  /* 0x44e0 */
> > > > compatible = "simple-pm-bus";
> > > >
> > > > target-module@0 { /* 0x44e0, ap 8 58.0 */
> > > > compatible = "ti,sysc-omap4", "ti,sysc";
> > > >
> > > > prcm: prcm@0 {
> > > > compatible = "ti,am3-prcm", "simple-bus";
> > > >
> > > > prm_per: prm@c00 { <= Actual Supplier
> > > > compatible = "ti,am3-prm-inst", 
> > > > "ti,omap-prm-inst";
> > > > };
> > > > };
> > > > };
> > > > };
> > > > };
> > > >  

Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

2022-06-30 Thread Tony Lindgren
* Saravana Kannan  [220630 23:25]:
> On Thu, Jun 30, 2022 at 4:26 PM Rob Herring  wrote:
> >
> > On Thu, Jun 30, 2022 at 5:11 PM Saravana Kannan  
> > wrote:
> > >
> > > On Mon, Jun 27, 2022 at 2:10 AM Tony Lindgren  wrote:
> > > >
> > > > * Saravana Kannan  [220623 08:17]:
> > > > > On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren  
> > > > > wrote:
> > > > > >
> > > > > > * Saravana Kannan  [220622 19:05]:
> > > > > > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren  
> > > > > > > wrote:
> > > > > > > > This issue is no directly related fw_devlink. It is a side 
> > > > > > > > effect of
> > > > > > > > removing driver_deferred_probe_check_state(). We no longer 
> > > > > > > > return
> > > > > > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > > > > > >
> > > > > > > Yes, I understand the issue. But 
> > > > > > > driver_deferred_probe_check_state()
> > > > > > > was deleted because fw_devlink=on should have short circuited the
> > > > > > > probe attempt with an  -EPROBE_DEFER before reaching the 
> > > > > > > bus/driver
> > > > > > > probe function and hitting this -ENOENT failure. That's why I was
> > > > > > > asking the other questions.
> > > > > >
> > > > > > OK. So where is the -EPROBE_DEFER supposed to happen without
> > > > > > driver_deferred_probe_check_state() then?
> > > > >
> > > > > device_links_check_suppliers() call inside really_probe() would short
> > > > > circuit and return an -EPROBE_DEFER if the device links are created as
> > > > > expected.
> > > >
> > > > OK
> > > >
> > > > > > Hmm so I'm not seeing any supplier for the top level ocp device in
> > > > > > the booting case without your patches. I see the suppliers for the
> > > > > > ocp child device instances only.
> > > > >
> > > > > Hmmm... this is strange (that the device link isn't there), but this
> > > > > is what I suspected.
> > > >
> > > > Yup, maybe it's because of the supplier being a device in the child
> > > > interconnect for the ocp.
> > >
> > > Ugh... yeah, this is why the normal (not SYNC_STATE_ONLY) device link
> > > isn't being created.
> > >
> > > So the aggregated view is something like (I had to set tabs = 4 space
> > > to fit it within 80 cols):
> > >
> > > ocp: ocp { <= Consumer
> > > compatible = "simple-pm-bus";
> > > power-domains = <&prm_per>; <=== Supplier ref
> > >
> > > l4_wkup: interconnect@44c0 {
> > > compatible = "ti,am33xx-l4-wkup", "simple-pm-bus";
> > >
> > > segment@20 {  /* 0x44e0 */
> > > compatible = "simple-pm-bus";
> > >
> > > target-module@0 { /* 0x44e0, ap 8 58.0 */
> > > compatible = "ti,sysc-omap4", "ti,sysc";
> > >
> > > prcm: prcm@0 {
> > > compatible = "ti,am3-prcm", "simple-bus";
> > >
> > > prm_per: prm@c00 { <= Actual Supplier
> > > compatible = "ti,am3-prm-inst", 
> > > "ti,omap-prm-inst";
> > > };
> > > };
> > > };
> > > };
> > > };
> > > };
> > >
> > > The power-domain supplier is the great-great-great-grand-child of the
> > > consumer. It's not clear to me how this is valid. What does it even
> > > mean?
> > >
> > > Rob, is this considered a valid DT?
> >
> > Valid DT for broken h/w.
> 
> I'm not sure even in that case it's valid. When the parent device is
> in reset (when the SoC is coming out of reset), there's no way the
> descendant is functional. And if the descendant is not functional, how
> is the parent device powered up? This just feels like an incorrect
> representation of the real h/w.

It should be correct representation based on scanning the interconnects
and looking at the documentation. Some interconnect parts are wired
always-on and some interconnect instances may be dual-mapped.

We have a quirk to probe prm/prcm first with pdata_quirks_init_clocks().
Maybe that also now fails in addition to the top level interconnect
probing no longer producing -EPROBE_DEFER.

> > So the domain must be default on and then simple-pm-bus is going to
> > hold a reference to the domain preventing it from ever getting powered
> > off and things seem to work. Except what happens during suspend?
> 
> But how can simple-pm-bus even get a reference? The PM domain can't
> get added until we are well into the probe of the simple-pm-bus and
> AFAICT the genpd attach is done before the driver probe is even
> called.

The prm/prcm gets of_platform_populate() called on it early.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

2022-06-27 Thread Tony Lindgren
* Saravana Kannan  [220623 08:17]:
> On Thu, Jun 23, 2022 at 12:01 AM Tony Lindgren  wrote:
> >
> > * Saravana Kannan  [220622 19:05]:
> > > On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren  wrote:
> > > > This issue is no directly related fw_devlink. It is a side effect of
> > > > removing driver_deferred_probe_check_state(). We no longer return
> > > > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> > >
> > > Yes, I understand the issue. But driver_deferred_probe_check_state()
> > > was deleted because fw_devlink=on should have short circuited the
> > > probe attempt with an  -EPROBE_DEFER before reaching the bus/driver
> > > probe function and hitting this -ENOENT failure. That's why I was
> > > asking the other questions.
> >
> > OK. So where is the -EPROBE_DEFER supposed to happen without
> > driver_deferred_probe_check_state() then?
> 
> device_links_check_suppliers() call inside really_probe() would short
> circuit and return an -EPROBE_DEFER if the device links are created as
> expected.

OK

> > Hmm so I'm not seeing any supplier for the top level ocp device in
> > the booting case without your patches. I see the suppliers for the
> > ocp child device instances only.
> 
> Hmmm... this is strange (that the device link isn't there), but this
> is what I suspected.

Yup, maybe it's because of the supplier being a device in the child
interconnect for the ocp.

> Now we need to figure out why it's missing. There are only a few
> things that could cause this and I don't see any of those. I already
> checked to make sure the power domain in this instance had a proper
> driver with a probe() function -- if it didn't, then that's one thing
> that'd could have caused the missing device link. The device does seem
> to have a proper driver, so looks like I can rule that out.
> 
> Can you point me to the dts file that corresponds to the specific
> board you are testing this one? I probably won't find anything, but I
> want to rule out some of the possibilities.

You can use the beaglebone black dts for example, that's
arch/arm/boot/dts/am335x-boneblack.dts and uses am33xx.dtsi for
ocp interconnect with simple-pm-bus.

> All the device link creation logic is inside drivers/base/core.c. So
> if you can look at the existing messages or add other stuff to figure
> out why the device link isn't getting created, that'd be handy. In
> either case, I'll continue staring at the DT and code to see what
> might be happening here.

In device_links_check_suppliers() I see these ocp suppliers:

platform ocp: device_links_check_suppliers: 1024: supplier 44e00d00.prm: 
link->status: 0 link->flags: 01c0
platform ocp: device_links_check_suppliers: 1024: supplier 44e01000.prm: 
link->status: 0 link->flags: 01c0
platform ocp: device_links_check_suppliers: 1024: supplier 44e00c00.prm: 
link->status: 0 link->flags: 01c0
platform ocp: device_links_check_suppliers: 1024: supplier 44e00e00.prm: 
link->status: 0 link->flags: 01c0
platform ocp: device_links_check_suppliers: 1024: supplier 44e01100.prm: 
link->status: 0 link->flags: 01c0
platform ocp: device_links_check_suppliers: 1024: supplier fixedregulator0: 
link->status: 1 link->flags: 01c0

No -EPROBE_DEFER is returned in device_links_check_suppliers() for
44e00c00.prm supplier for beaglebone black for example, 0 gets
returned.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

2022-06-23 Thread Tony Lindgren
* Saravana Kannan  [220622 19:05]:
> On Tue, Jun 21, 2022 at 9:59 PM Tony Lindgren  wrote:
> >
> > Hi,
> >
> > * Saravana Kannan  [220621 19:29]:
> > > On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren  wrote:
> > > >
> > > > Hi,
> > > >
> > > > * Saravana Kannan  [700101 02:00]:
> > > > > Now that fw_devlink=on by default and fw_devlink supports
> > > > > "power-domains" property, the execution will never get to the point
> > > > > where driver_deferred_probe_check_state() is called before the 
> > > > > supplier
> > > > > has probed successfully or before deferred probe timeout has expired.
> > > > >
> > > > > So, delete the call and replace it with -ENODEV.
> > > >
> > > > Looks like this causes omaps to not boot in Linux next.
> > >
> > > Can you please point me to an example DTS I could use for debugging
> > > this? I'm assuming you are leaving fw_devlink=on and not turning it
> > > off or putting it in permissive mode.
> >
> > Sure, this seems to happen at least with simple-pm-bus as the top
> > level interconnect with a configured power-domains property:
> >
> > $ git grep -A10 "ocp {" arch/arm/boot/dts/*.dtsi | grep -B3 -A4 
> > simple-pm-bus
> 
> Thanks for the example. I generally start looking from dts (not dtsi)
> files in case there are some DT property override/additions after the
> dtsi files are included in the dts file. But I'll assume for now
> that's not the case. If there's a specific dts file for a board I can
> look from that'd be helpful to rule out those kinds of issues.
> 
> For now, I looked at arch/arm/boot/dts/omap4.dtsi.

OK it should be very similar for all the affected SoCs.

> > This issue is no directly related fw_devlink. It is a side effect of
> > removing driver_deferred_probe_check_state(). We no longer return
> > -EPROBE_DEFER at the end of driver_deferred_probe_check_state().
> 
> Yes, I understand the issue. But driver_deferred_probe_check_state()
> was deleted because fw_devlink=on should have short circuited the
> probe attempt with an  -EPROBE_DEFER before reaching the bus/driver
> probe function and hitting this -ENOENT failure. That's why I was
> asking the other questions.

OK. So where is the -EPROBE_DEFER supposed to happen without
driver_deferred_probe_check_state() then?

> > > > On platform_probe() genpd_get_from_provider() returns
> > > > -ENOENT.
> > >
> > > This error is with the series I assume?
> >
> > On the first probe genpd_get_from_provider() will return -ENOENT in
> > both cases. The list is empty on the first probe and there are no
> > genpd providers at this point.
> >
> > Earlier with driver_deferred_probe_check_state(), the initial -ENOENT
> > ends up getting changed to -EPROBE_DEFER at the end of
> > driver_deferred_probe_check_state(), we are now missing that.
> 
> Right, I was aware -ENOENT would be returned if we got this far. But
> the point of this series is that you shouldn't have gotten that far
> before your pm domain device is ready. Hence my questions from the
> earlier reply.

OK

> Can I get answers to rest of my questions in the first reply please?
> That should help us figure out why fw_devlink let us get this far.
> Summarize them here to make it easy:
> * Are you running with fw_devlink=on?

Yes with the default with no specific kernel params so looks like
FW_DEVLINK_FLAGS_ON.

> * Is the"ti,omap4-prm-inst"/"ti,omap-prm-inst" built-in in this case?

Yes

> * If it's not built-in, can you please try deferred_probe_timeout=0
> and deferred_probe_timeout=30 and see if either one of them help?

It's built in so I did not try these.

> * Can I get the output of "ls -d supplier:*" and "cat
> supplier:*/status" output from the sysfs dir for the ocp device
> without this series where it boots properly.

Hmm so I'm not seeing any supplier for the top level ocp device in
the booting case without your patches. I see the suppliers for the
ocp child device instances only.

Without your patches I see simple-pm-bus probe initially with
EPROBE_DEFER like I described earlier, and then simple-pm-bus probes
on the second try.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

2022-06-21 Thread Tony Lindgren
Hi,

* Saravana Kannan  [220621 19:29]:
> On Tue, Jun 21, 2022 at 12:28 AM Tony Lindgren  wrote:
> >
> > Hi,
> >
> > * Saravana Kannan  [700101 02:00]:
> > > Now that fw_devlink=on by default and fw_devlink supports
> > > "power-domains" property, the execution will never get to the point
> > > where driver_deferred_probe_check_state() is called before the supplier
> > > has probed successfully or before deferred probe timeout has expired.
> > >
> > > So, delete the call and replace it with -ENODEV.
> >
> > Looks like this causes omaps to not boot in Linux next.
> 
> Can you please point me to an example DTS I could use for debugging
> this? I'm assuming you are leaving fw_devlink=on and not turning it
> off or putting it in permissive mode.

Sure, this seems to happen at least with simple-pm-bus as the top
level interconnect with a configured power-domains property:

$ git grep -A10 "ocp {" arch/arm/boot/dts/*.dtsi | grep -B3 -A4 simple-pm-bus

This issue is no directly related fw_devlink. It is a side effect of
removing driver_deferred_probe_check_state(). We no longer return
-EPROBE_DEFER at the end of driver_deferred_probe_check_state().

> > On platform_probe() genpd_get_from_provider() returns
> > -ENOENT.
> 
> This error is with the series I assume?

On the first probe genpd_get_from_provider() will return -ENOENT in
both cases. The list is empty on the first probe and there are no
genpd providers at this point.

Earlier with driver_deferred_probe_check_state(), the initial -ENOENT
ends up getting changed to -EPROBE_DEFER at the end of
driver_deferred_probe_check_state(), we are now missing that.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 1/9] PM: domains: Delete usage of driver_deferred_probe_check_state()

2022-06-21 Thread Tony Lindgren
Hi,

* Saravana Kannan  [700101 02:00]:
> Now that fw_devlink=on by default and fw_devlink supports
> "power-domains" property, the execution will never get to the point
> where driver_deferred_probe_check_state() is called before the supplier
> has probed successfully or before deferred probe timeout has expired.
> 
> So, delete the call and replace it with -ENODEV.

Looks like this causes omaps to not boot in Linux next. With this
simple-pm-bus fails to probe initially as the power-domain is not
yet available. On platform_probe() genpd_get_from_provider() returns
-ENOENT.

Seems like other stuff is potentially broken too, any ideas on
how to fix this?

Regards,

Tony



> 
> Signed-off-by: Saravana Kannan 
> ---
>  drivers/base/power/domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 739e52cd4aba..3e86772d5fac 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2730,7 +2730,7 @@ static int __genpd_dev_pm_attach(struct device *dev, 
> struct device *base_dev,
>   mutex_unlock(&gpd_list_lock);
>   dev_dbg(dev, "%s() failed to find PM domain: %ld\n",
>   __func__, PTR_ERR(pd));
> - return driver_deferred_probe_check_state(base_dev);
> + return -ENODEV;
>   }
>  
>   dev_dbg(dev, "adding to PM domain %s\n", pd->name);
> -- 
> 2.36.1.255.ge46751e96f-goog
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/omap: Fix regression in probe for NULL pointer dereference

2022-04-08 Thread Tony Lindgren
Hi,

* Joerg Roedel  [220408 08:23]:
> On Thu, Apr 07, 2022 at 08:39:05AM +0300, Tony Lindgren wrote:
> > Can you guys please get this fix into the -rc series? Or ack it so
> > I can pick it up into my fixes branch?
> 
> Sorry for the delay, Covid catched me so I was away from email for
> almost 2 week. This patch is picked-up now and I will send it upstream
> for -rc2.

OK welcome back then, and hopefully no serious symptoms. Thanks for
picking up the patch.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/omap: Fix regression in probe for NULL pointer dereference

2022-04-06 Thread Tony Lindgren
Hi,

* Tony Lindgren  [220331 09:21]:
> Commit 3f6634d997db ("iommu: Use right way to retrieve iommu_ops") started
> triggering a NULL pointer dereference for some omap variants:
> 
> __iommu_probe_device from probe_iommu_group+0x2c/0x38
> probe_iommu_group from bus_for_each_dev+0x74/0xbc
> bus_for_each_dev from bus_iommu_probe+0x34/0x2e8
> bus_iommu_probe from bus_set_iommu+0x80/0xc8
> bus_set_iommu from omap_iommu_init+0x88/0xcc
> omap_iommu_init from do_one_initcall+0x44/0x24
> 
> This is caused by omap iommu probe returning 0 instead of ERR_PTR(-ENODEV)
> as noted by Jason Gunthorpe .
> 
> Looks like the regression already happened with an earlier commit
> 6785eb9105e3 ("iommu/omap: Convert to probe/release_device() call-backs")
> that changed the function return type and missed converting one place.

Can you guys please get this fix into the -rc series? Or ack it so
I can pick it up into my fixes branch?

Without this fix booting is failing for a bunch of devices.

Regards,

Tony


> Cc: Drew Fustini 
> Cc: Lu Baolu 
> Cc: Suman Anna 
> Suggested-by: Jason Gunthorpe 
> Fixes: 6785eb9105e3 ("iommu/omap: Convert to probe/release_device() 
> call-backs")
> Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
> Signed-off-by: Tony Lindgren 
> ---
>  drivers/iommu/omap-iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -1661,7 +1661,7 @@ static struct iommu_device 
> *omap_iommu_probe_device(struct device *dev)
>   num_iommus = of_property_count_elems_of_size(dev->of_node, "iommus",
>sizeof(phandle));
>   if (num_iommus < 0)
> - return 0;
> + return ERR_PTR(-ENODEV);
>  
>   arch_data = kcalloc(num_iommus + 1, sizeof(*arch_data), GFP_KERNEL);
>   if (!arch_data)
> -- 
> 2.35.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 8/9] iommu: Remove unused argument in is_attach_deferred

2022-03-30 Thread Tony Lindgren
Hi,

* Jason Gunthorpe  [220330 17:31]:
> On Wed, Mar 30, 2022 at 08:19:37PM +0300, Tony Lindgren wrote:
> 
> > > > __iommu_probe_device from probe_iommu_group+0x2c/0x38
> > > > probe_iommu_group from bus_for_each_dev+0x74/0xbc
> > > > bus_for_each_dev from bus_iommu_probe+0x34/0x2e8
> > > > bus_iommu_probe from bus_set_iommu+0x80/0xc8
> > > > bus_set_iommu from omap_iommu_init+0x88/0xcc
> > > > omap_iommu_init from do_one_initcall+0x44/0x24c
> > > > 
> > > > Any ideas for a fix?
> > > > 
> > > > It would be good to fix this quickly so we don't end up with a broken
> > > > v5.18-rc1..
> > > > 
> > > > For reference, this is mainline commit 41bb23e70b50 ("iommu: Remove 
> > > > unused
> > > > argument in is_attach_deferred").
> > > 
> > > Are you confident in the bisection? I don't see how that commit could
> > > NULL deref..
> > 
> > Oops sorry you're right, the breaking commit is a different patch, it's
> > 3f6634d997db ("iommu: Use right way to retrieve iommu_ops") instead. I must
> > have picked the wrong commit while testing.
> 
> That makes alot more sense
>  
> > > Can you find the code that is the NULL deref?
> > 
> > I can debug a bit more tomorrow.
> 
> Looks like omap's omap_iommu_probe_device() is buggy, it returns 0 on
> error paths:
> 
>   num_iommus = of_property_count_elems_of_size(dev->of_node, "iommus",
>sizeof(phandle));
>   if (num_iommus < 0)
>   return 0;
> 
> This function needs to return an errno -ENODEV
> 
> Otherwise it causes a NULL dev->iommu->iommu_dev and dev_iommu_ops() will
> crash.

You got it. This fixes the issue for me. Looks like the regression already
happened earlier and recent changes just expose it.

For reference, fix posted at:

https://lore.kernel.org/linux-iommu/20220331062301.24269-1-t...@atomide.com/T/#u

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/omap: Fix regression in probe for NULL pointer dereference

2022-03-30 Thread Tony Lindgren
Commit 3f6634d997db ("iommu: Use right way to retrieve iommu_ops") started
triggering a NULL pointer dereference for some omap variants:

__iommu_probe_device from probe_iommu_group+0x2c/0x38
probe_iommu_group from bus_for_each_dev+0x74/0xbc
bus_for_each_dev from bus_iommu_probe+0x34/0x2e8
bus_iommu_probe from bus_set_iommu+0x80/0xc8
bus_set_iommu from omap_iommu_init+0x88/0xcc
omap_iommu_init from do_one_initcall+0x44/0x24

This is caused by omap iommu probe returning 0 instead of ERR_PTR(-ENODEV)
as noted by Jason Gunthorpe .

Looks like the regression already happened with an earlier commit
6785eb9105e3 ("iommu/omap: Convert to probe/release_device() call-backs")
that changed the function return type and missed converting one place.

Cc: Drew Fustini 
Cc: Lu Baolu 
Cc: Suman Anna 
Suggested-by: Jason Gunthorpe 
Fixes: 6785eb9105e3 ("iommu/omap: Convert to probe/release_device() call-backs")
Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
Signed-off-by: Tony Lindgren 
---
 drivers/iommu/omap-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1661,7 +1661,7 @@ static struct iommu_device 
*omap_iommu_probe_device(struct device *dev)
num_iommus = of_property_count_elems_of_size(dev->of_node, "iommus",
 sizeof(phandle));
if (num_iommus < 0)
-   return 0;
+   return ERR_PTR(-ENODEV);
 
arch_data = kcalloc(num_iommus + 1, sizeof(*arch_data), GFP_KERNEL);
if (!arch_data)
-- 
2.35.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 8/9] iommu: Remove unused argument in is_attach_deferred

2022-03-30 Thread Tony Lindgren
* Jason Gunthorpe  [220330 14:21]:
> On Wed, Mar 30, 2022 at 05:00:39PM +0300, Tony Lindgren wrote:
> > Hi,
> > 
> > * Lu Baolu  [700101 02:00]:
> > > The is_attach_deferred iommu_ops callback is a device op. The domain
> > > argument is unnecessary and never used. Remove it to make code clean.
> > 
> > Looks like this causes a regression for at least drivers/iommu/omap-iommu.c.
> > 
> > To me it seems the issue is there is no is_attach_deferred implemented, so
> > we get a NULL pointer dereference at virtual address 0008:
> > 
> > __iommu_probe_device from probe_iommu_group+0x2c/0x38
> > probe_iommu_group from bus_for_each_dev+0x74/0xbc
> > bus_for_each_dev from bus_iommu_probe+0x34/0x2e8
> > bus_iommu_probe from bus_set_iommu+0x80/0xc8
> > bus_set_iommu from omap_iommu_init+0x88/0xcc
> > omap_iommu_init from do_one_initcall+0x44/0x24c
> > 
> > Any ideas for a fix?
> > 
> > It would be good to fix this quickly so we don't end up with a broken
> > v5.18-rc1..
> > 
> > For reference, this is mainline commit 41bb23e70b50 ("iommu: Remove unused
> > argument in is_attach_deferred").
> 
> Are you confident in the bisection? I don't see how that commit could
> NULL deref..

Oops sorry you're right, the breaking commit is a different patch, it's
3f6634d997db ("iommu: Use right way to retrieve iommu_ops") instead. I must
have picked the wrong commit while testing.

> Can you find the code that is the NULL deref?

I can debug a bit more tomorrow.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 8/9] iommu: Remove unused argument in is_attach_deferred

2022-03-30 Thread Tony Lindgren
Hi,

* Lu Baolu  [700101 02:00]:
> The is_attach_deferred iommu_ops callback is a device op. The domain
> argument is unnecessary and never used. Remove it to make code clean.

Looks like this causes a regression for at least drivers/iommu/omap-iommu.c.

To me it seems the issue is there is no is_attach_deferred implemented, so
we get a NULL pointer dereference at virtual address 0008:

__iommu_probe_device from probe_iommu_group+0x2c/0x38
probe_iommu_group from bus_for_each_dev+0x74/0xbc
bus_for_each_dev from bus_iommu_probe+0x34/0x2e8
bus_iommu_probe from bus_set_iommu+0x80/0xc8
bus_set_iommu from omap_iommu_init+0x88/0xcc
omap_iommu_init from do_one_initcall+0x44/0x24c

Any ideas for a fix?

It would be good to fix this quickly so we don't end up with a broken
v5.18-rc1..

For reference, this is mainline commit 41bb23e70b50 ("iommu: Remove unused
argument in is_attach_deferred").

Regards,

Tony

#regzbot ^introduced 41bb23e70b50
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] ARM/omap1: switch to use dma_direct_set_offset for lbus DMA offsets

2020-09-20 Thread Tony Lindgren
* Janusz Krzysztofik  [200919 22:29]:
> Hi Tony,
> 
> On Friday, September 18, 2020 7:49:33 A.M. CEST Tony Lindgren wrote:
> > * Christoph Hellwig  [200917 17:37]:
> > > Switch the omap1510 platform ohci device to use dma_direct_set_offset
> > > to set the DMA offset instead of using direct hooks into the DMA
> > > mapping code and remove the now unused hooks.
> > 
> > Looks nice to me :) I still can't test this probably for few more weeks
> > though but hopefully Aaro or Janusz (Added to Cc) can test it.
> 
> Works for me on Amstrad Delta (tested with a USB ethernet adapter).
> 
> Tested-by: Janusz Krzysztofik 

Great, good to hear! And thanks for testing it.

Christoph, feel free to queue this along with the other patches:

Acked-by: Tony Lindgren 

Or let me know if you want me to pick it up.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] ARM/omap1: switch to use dma_direct_set_offset for lbus DMA offsets

2020-09-17 Thread Tony Lindgren
* Christoph Hellwig  [200917 17:37]:
> Switch the omap1510 platform ohci device to use dma_direct_set_offset
> to set the DMA offset instead of using direct hooks into the DMA
> mapping code and remove the now unused hooks.

Looks nice to me :) I still can't test this probably for few more weeks
though but hopefully Aaro or Janusz (Added to Cc) can test it.

Regards,

Tony

> Signed-off-by: Christoph Hellwig 
> ---
>  arch/arm/include/asm/dma-direct.h | 18 -
>  arch/arm/mach-omap1/include/mach/memory.h | 31 ---
>  arch/arm/mach-omap1/usb.c | 22 
>  3 files changed, 22 insertions(+), 49 deletions(-)
> 
> diff --git a/arch/arm/include/asm/dma-direct.h 
> b/arch/arm/include/asm/dma-direct.h
> index 436544aeb83405..77fcb7ee5ec907 100644
> --- a/arch/arm/include/asm/dma-direct.h
> +++ b/arch/arm/include/asm/dma-direct.h
> @@ -9,7 +9,6 @@
>   * functions used internally by the DMA-mapping API to provide DMA
>   * addresses. They must not be used by drivers.
>   */
> -#ifndef __arch_pfn_to_dma
>  static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
>  {
>   if (dev && dev->dma_range_map)
> @@ -34,23 +33,6 @@ static inline dma_addr_t virt_to_dma(struct device *dev, 
> void *addr)
>   return (dma_addr_t)__virt_to_bus((unsigned long)(addr));
>  }
>  
> -#else
> -static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
> -{
> - return __arch_pfn_to_dma(dev, pfn);
> -}
> -
> -static inline unsigned long dma_to_pfn(struct device *dev, dma_addr_t addr)
> -{
> - return __arch_dma_to_pfn(dev, addr);
> -}
> -
> -static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
> -{
> - return __arch_virt_to_dma(dev, addr);
> -}
> -#endif
> -
>  static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
>  {
>   unsigned int offset = paddr & ~PAGE_MASK;
> diff --git a/arch/arm/mach-omap1/include/mach/memory.h 
> b/arch/arm/mach-omap1/include/mach/memory.h
> index 1142560e0078f5..36bccb6ab8 100644
> --- a/arch/arm/mach-omap1/include/mach/memory.h
> +++ b/arch/arm/mach-omap1/include/mach/memory.h
> @@ -14,42 +14,11 @@
>   * OMAP-1510 bus address is translated into a Local Bus address if the
>   * OMAP bus type is lbus. We do the address translation based on the
>   * device overriding the defaults used in the dma-mapping API.
> - * Note that the is_lbus_device() test is not very efficient on 1510
> - * because of the strncmp().
>   */
> -#if defined(CONFIG_ARCH_OMAP15XX) && !defined(__ASSEMBLER__)
>  
>  /*
>   * OMAP-1510 Local Bus address offset
>   */
>  #define OMAP1510_LB_OFFSET   UL(0x3000)
>  
> -#define virt_to_lbus(x)  ((x) - PAGE_OFFSET + OMAP1510_LB_OFFSET)
> -#define lbus_to_virt(x)  ((x) - OMAP1510_LB_OFFSET + PAGE_OFFSET)
> -#define is_lbus_device(dev)  (cpu_is_omap15xx() && dev && 
> (strncmp(dev_name(dev), "ohci", 4) == 0))
> -
> -#define __arch_pfn_to_dma(dev, pfn)  \
> - ({ dma_addr_t __dma = __pfn_to_phys(pfn); \
> -if (is_lbus_device(dev)) \
> - __dma = __dma - PHYS_OFFSET + OMAP1510_LB_OFFSET; \
> -__dma; })
> -
> -#define __arch_dma_to_pfn(dev, addr) \
> - ({ dma_addr_t __dma = addr; \
> -if (is_lbus_device(dev)) \
> - __dma += PHYS_OFFSET - OMAP1510_LB_OFFSET;  \
> -__phys_to_pfn(__dma);\
> - })
> -
> -#define __arch_dma_to_virt(dev, addr)({ (void *) 
> (is_lbus_device(dev) ? \
> - lbus_to_virt(addr) : \
> - __phys_to_virt(addr)); })
> -
> -#define __arch_virt_to_dma(dev, addr)({ unsigned long __addr = 
> (unsigned long)(addr); \
> -(dma_addr_t) (is_lbus_device(dev) ? \
> - virt_to_lbus(__addr) : \
> - __virt_to_phys(__addr)); })
> -
> -#endif   /* CONFIG_ARCH_OMAP15XX */
> -
>  #endif
> diff --git a/arch/arm/mach-omap1/usb.c b/arch/arm/mach-omap1/usb.c
> index d8e9bbda8f7bdd..ba8566204ea9f4 100644
> --- a/arch/arm/mach-omap1/usb.c
> +++ b/arch/arm/mach-omap1/usb.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -542,6 +543,25 @@ static u32 __init omap1_usb2_init(unsigned nwires, 
> unsigned alt_pingroup)
>  /* ULPD_APLL_CTRL */
>  #define APLL_NDPLL_SWITCH(1 << 0)
>  
> +static int omap_1510_usb_ohci_notifier(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct device *dev = data;
> +
> + if (event != BUS_NOTIFY_ADD_DEVICE)
> + return NOTIFY_DONE;
> +
> + if (strncmp(dev_name(dev), "ohci", 4) == 0 &&
> + dma_direct_set_offset(dev, PHYS_OFFSET, OMAP1510_LB_OFFSET,
> + (u64)-1))
> +

Re: [PATCH 0/2] OMAP IOMMU fixes to go with 5.4 OMAP IOMMU changes

2019-10-18 Thread Tony Lindgren
* Suman Anna  [191017 23:00]:
> Hi Tony,
> 
> On 8/26/19 7:14 PM, Suman Anna wrote:
> > Hi Tony,
> > 
> > The following 2 patches need to go along with the recent "iommu/omap: misc
> > fixes" series [1] that is currently staged [2] for a 5.4 merge and available
> > in linux-next. That series added runtime pm callbacks in preparation for
> > the ti-sysc migration, but without adding the necessary platform data
> > callbacks for the existing functional MMUs on OMAP3, OMAP4 and OMAP5 SoCs.
> > These 2 patches add the same to maintain the functionality (l3_noc errors
> > are the visible symptom while enabling the MMUs without these patches).
> > 
> > OMAP4 and OMAP5 would also need another set of fixes related to the
> > breakage caused by the switch to clkctrl clocks as seen in [3].
> > 
> > These patches do have a dependency on the staged patches, so either you
> > submit a pull-request towards 5.4-rc2 using 5.4-rc1 baseline, or let
> > Joerg pick these through the arm/omap IOMMU branch for 5.4-rc1 itself.
> > 
> > Sakari/Laurent,
> > Appreciate it if you can check the OMAP3 ISP functionality on latest
> > linux-next with the second patch. I have verified the ISP MMU programming
> > only through an unit-test.
> > 
> > Tero,
> > I will be submitting another patch against OMAP IOMMU driver to skip the
> > pdata callbacks for ti-sysc in the next couple of days.
> > 
> > regards
> > Suman
> > 
> > [1] 
> > https://lore.kernel.org/linux-iommu/20190809153730.gf12...@8bytes.org/T/#mec99f8e8ed351689f4fcc76f4f000f9144a02b51
> > [2] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/log/?h=arm/omap
> > [3] https://patchwork.kernel.org/patch/11082125/#22829477
> > 
> > Suman Anna (2):
> >   ARM: OMAP2+: Plug in device_enable/idle ops for IOMMUs
> >   ARM: OMAP2+: Add pdata for OMAP3 ISP IOMMU
> 
> Can you please pick these patches up for 5.4-rc cycle? The OMAP IOMMU
> changes that went in for 5.4-rc1 need the pdata to be plugged in.

Oh OK. Sorry for missing these, I untagged them earlier as they
produced kbuilder test failures as the dependencies were
missing earlier.

Applying both into fixes.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] Revert "dma-contiguous: do not allocate a single page from CMA area"

2019-02-26 Thread Tony Lindgren
* Robin Murphy  [190226 23:36]:
> On 2019-02-26 8:23 pm, Nicolin Chen wrote:
> > This reverts commit d222e42e88168fd67e6d131984b86477af1fc256.
> > 
> > The original change breaks omap dss:
> >  omapdss_dispc 58001000.dispc:
> >  dispc_errata_i734_wa_init: dma_alloc_writecombine failed
> > 
> > Let's revert it first and then find a safer solution instead.

Sounds like a good idea since we're only have few days left
before the merge window.

> Ah, I think I see the problem - once arch/arm's __dma_alloc() has decided to
> use CMA (because dev_get_cma_area(dev) returns the global area), it then
> won't fall back to trying a regular page allocation if
> dma_alloc_from_contiguous() returns NULL. Thus anything on 32-bit Arm trying
> to allocate a single-page buffer in blockable context with a CMA-enabled
> config is just going to fail. Similarly, it looks like none of the
> DMA_ATTR_FORCE_CONTIGUOUS cases are prepared to handle this change either
> (amd_iommu appears technically affected, but is already using
> dma_alloc_from_contiguous() backwards compared to everyone else, hmm).
> 
> I guess the question is whether to add alloc_page()/free_page() fallbacks to
> those call sites, or stuff them directly into the CMA helpers here.

Well if you come up with some test patch, I can easily test it :)

> > Would you please test and verify? Thanks!

Yes this revert works for me:

Tested-by: Tony Lindgren 

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 06/14] mmc: omap: handle highmem pages

2019-02-12 Thread Tony Lindgren
* Christoph Hellwig  [190212 07:27]:
> Instead of setting up a kernel pointer to track the current PIO address,
> track the offset in the current page, and do an atomic kmap for the page
> while doing the actual PIO operations.

I'm currently having issues booting my test devices (770 and n8x0)
with this MMC controller so I can't test these patches currently.
Aaro, can you please test when you have a chance?

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 0/4] Cleanup legacy OMAP IOMMU device creation

2015-11-30 Thread Tony Lindgren
* Suman Anna  [151022 10:16]:
> Hi Tony,
> 
> On 09/16/2015 06:48 PM, Suman Anna wrote:
> > Hi Tony,
> > 
> > The following series removes the legacy platform device creation
> > logic for OMAP IOMMU devices. I will cleanup the legacy support
> > from the OMAP IOMMU driver in a subsequent merge window after
> > this series makes it to mainline.
> > 
> > Patches are based on 4.3-rc1 + the OMAP3 ISP instantiation cleanup
> > patch [1]. All the patches need to be picked up sequentially,
> > otherwise a NULL pointer dereference crash might be seen on OMAP3
> > legacy boots as the dev attribute structure is deferenced directly
> > in mach-omap2/omap-iommu.c during platform data creation. Also, the
> > last patch removes the structure definition altogether, so will
> > cause build issues if picked separately from the hwmod cleanup
> > patches.
> > 
> > I do not have any boards where I can still perform a legacy-style
> > boot, so patches verified using DT-boot only.
> > 
> > regards
> > Suman
> > 
> > [1] https://patchwork.kernel.org/patch/6806891/
> > 
> > Suman Anna (4):
> >   ARM: OMAP2+: Remove legacy device instantiation of IOMMUs
> >   ARM: OMAP3: hwmod data: Remove legacy IOMMU data
> >   ARM: OMAP4: hwmod data: Remove legacy IOMMU attr and addrs
> >   ARM: OMAP2+: Remove omap_mmu_dev_attr structure
> 
> Ping on this series. You should be able to pick up atleast patch 1 if
> not picking all, now that the OMAP3 ISP cleanup patch is staged in your
> omap-for-4.4/cleanup branch.

Sorry for the delays, applying this series into omap-for-v4.5/soc.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [REPOST PATCH 1/4] ARM: dts: DRA7: Add dsp1_system syscon node

2015-10-12 Thread Tony Lindgren
* Tony Lindgren  [151012 15:57]:
> * Suman Anna  [151012 15:37]:
> > Hi Tony,
> > 
> > On 10/12/2015 04:43 PM, Tony Lindgren wrote:
> > > * Suman Anna  [151002 16:27]:
> > >> The DSP_SYSTEM sub-module is a dedicated system control logic
> > >> module present within a DRA7 DSP processor sub-system. This
> > >> module is responsible for power management, clock generation
> > >> and connection to the device PRCM module.
> > >>
> > >> Add a syscon node for this module for the DSP1 processor
> > >> sub-system. This is added as a syscon node as it is a common
> > >> configuration module that can be used by the different IOMMU
> > >> instances and the corresponding remoteproc device.
> > >>
> > >> The node is added to the common dra7.dtsi file, as the DSP1
> > >> processor sub-system is mostly common across all the variants
> > >> of the DRA7 SoC family.
> > >>
> > >> Signed-off-by: Suman Anna 
> > >> ---
> > >>  arch/arm/boot/dts/dra7.dtsi | 5 +
> > >>  1 file changed, 5 insertions(+)
> > >>
> > >> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> > >> index e289c706d27d..62055094e8d5 100644
> > >> --- a/arch/arm/boot/dts/dra7.dtsi
> > >> +++ b/arch/arm/boot/dts/dra7.dtsi
> > >> @@ -292,6 +292,11 @@
> > >>  #thermal-sensor-cells = <1>;
> > >>  };
> > >>  
> > >> +dsp1_system: dsp_system@40d0 {
> > >> +compatible = "syscon";
> > >> +reg = <0x40d0 0x100>;
> > >> +};
> > >> +
> > >>  sdma: dma-controller@4a056000 {
> > >>  compatible = "ti,omap4430-sdma";
> > >>  reg = <0x4a056000 0x1000>;
> > > 
> > > Hmm so why would you want to set up a complete device as a syscon
> > > mapping rather than just doing ioremap on it?
> > > 
> > > What drivers will be sharing access to these registers?
> > 
> > Two different instances of the MMU for now, both get probed
> > independently. But there are other registers which a remoteproc driver
> > will mostly be interested in (like DSP_SYS_STAT for knowing the C66x
> > idle/active status).
> 
> OK makes sense to me then.

And applying these into omap-for-v4.4/dt.

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [REPOST PATCH 1/4] ARM: dts: DRA7: Add dsp1_system syscon node

2015-10-12 Thread Tony Lindgren
* Suman Anna  [151012 15:37]:
> Hi Tony,
> 
> On 10/12/2015 04:43 PM, Tony Lindgren wrote:
> > * Suman Anna  [151002 16:27]:
> >> The DSP_SYSTEM sub-module is a dedicated system control logic
> >> module present within a DRA7 DSP processor sub-system. This
> >> module is responsible for power management, clock generation
> >> and connection to the device PRCM module.
> >>
> >> Add a syscon node for this module for the DSP1 processor
> >> sub-system. This is added as a syscon node as it is a common
> >> configuration module that can be used by the different IOMMU
> >> instances and the corresponding remoteproc device.
> >>
> >> The node is added to the common dra7.dtsi file, as the DSP1
> >> processor sub-system is mostly common across all the variants
> >> of the DRA7 SoC family.
> >>
> >> Signed-off-by: Suman Anna 
> >> ---
> >>  arch/arm/boot/dts/dra7.dtsi | 5 +
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> >> index e289c706d27d..62055094e8d5 100644
> >> --- a/arch/arm/boot/dts/dra7.dtsi
> >> +++ b/arch/arm/boot/dts/dra7.dtsi
> >> @@ -292,6 +292,11 @@
> >>#thermal-sensor-cells = <1>;
> >>};
> >>  
> >> +  dsp1_system: dsp_system@40d0 {
> >> +  compatible = "syscon";
> >> +  reg = <0x40d0 0x100>;
> >> +  };
> >> +
> >>sdma: dma-controller@4a056000 {
> >>compatible = "ti,omap4430-sdma";
> >>reg = <0x4a056000 0x1000>;
> > 
> > Hmm so why would you want to set up a complete device as a syscon
> > mapping rather than just doing ioremap on it?
> > 
> > What drivers will be sharing access to these registers?
> 
> Two different instances of the MMU for now, both get probed
> independently. But there are other registers which a remoteproc driver
> will mostly be interested in (like DSP_SYS_STAT for knowing the C66x
> idle/active status).

OK makes sense to me then.

Thanks,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [REPOST PATCH 1/4] ARM: dts: DRA7: Add dsp1_system syscon node

2015-10-12 Thread Tony Lindgren
* Suman Anna  [151002 16:27]:
> The DSP_SYSTEM sub-module is a dedicated system control logic
> module present within a DRA7 DSP processor sub-system. This
> module is responsible for power management, clock generation
> and connection to the device PRCM module.
> 
> Add a syscon node for this module for the DSP1 processor
> sub-system. This is added as a syscon node as it is a common
> configuration module that can be used by the different IOMMU
> instances and the corresponding remoteproc device.
> 
> The node is added to the common dra7.dtsi file, as the DSP1
> processor sub-system is mostly common across all the variants
> of the DRA7 SoC family.
> 
> Signed-off-by: Suman Anna 
> ---
>  arch/arm/boot/dts/dra7.dtsi | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index e289c706d27d..62055094e8d5 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -292,6 +292,11 @@
>   #thermal-sensor-cells = <1>;
>   };
>  
> + dsp1_system: dsp_system@40d0 {
> + compatible = "syscon";
> + reg = <0x40d0 0x100>;
> + };
> +
>   sdma: dma-controller@4a056000 {
>   compatible = "ti,omap4430-sdma";
>   reg = <0x4a056000 0x1000>;

Hmm so why would you want to set up a complete device as a syscon
mapping rather than just doing ioremap on it?

What drivers will be sharing access to these registers?

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx

2015-09-23 Thread Tony Lindgren
* Suman Anna  [150903 16:01]:
> >>>> On 07/23/2015 02:24 AM, Tony Lindgren wrote:
> > OK maybe check the syss/sysc registers involved here for each hardware
> > module here and which driver tinkers with which registers? This will
> > make things a lot easier in the long run for sure.
> 
> The OMAP remoteproc driver typically deals with only the reset registers
> and another register in the Control module for programming the boot
> address for the DSPs. You can look up the OMAP4 hwmod data for
> reference. There is currently a .prcm.omap4.modulemode set for these,
> and I have a patch to remove those. I haven't yet posted it yet [1],
> because OMAP remoteprocs are still not bootable on mainline.

Thanks for checking that. Like we chatted, let's make sure we don't
end up with iommu and remoteproc "consumer device" specific data
in the iommu and remoteproc code.

Regards,

Tony

> [1]
> http://git.ti.com/gitweb/?p=rpmsg/remoteproc.git;a=commit;h=0dd2a6a2d8869ee6cec92f75d70663259a41c11f
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx

2015-07-27 Thread Tony Lindgren
* Suman Anna  [150724 09:27]:
> Hi Tony,
> 
> On 07/23/2015 11:30 PM, Tony Lindgren wrote:
> > * Suman Anna  [150723 09:25]:
> >> Hi Tony,
> >>
> >> On 07/23/2015 02:24 AM, Tony Lindgren wrote:
> >>> * Suman Anna  [150722 09:25]:
> >>>> On 07/22/2015 12:26 AM, Tony Lindgren wrote:
> >>>>>
> >>>>> I don't like using syscon for tinkering directly with SoC registers.
> >>>>
> >>>> This is not a SoC-level register, but a register within a sub-module of
> >>>> the DSP processor sub-system. The DSP_SYSTEM sub-module in general is
> >>>> described in Section 5.3.3 of the TRM [1], and it implements different
> >>>> functionalities like the PRCM handshaking, wakeup logic and DSP
> >>>> subsystem top-level configuration. It is a module present within the DSP
> >>>> processor sub-system, so can only be accessed when the sub-system is
> >>>> clocked and the appropriate reset is released.
> >>>
> >>> OK so if it's specific to the DSP driver along the lines of sysc and
> >>> syss registers.
> >>
> >> There will be those registers too within the MMU config register space,
> >> even for DRA7xx MMUs. This is different, think of it like a register in
> >> the Control module except that it is present within the DSP sub-system
> >> instead of at the SoC level.
> > 
> > And what is taking care of pm_runtime_get here to ensure the module
> > is powered and clocked?
> 
> pm_runtime_get_sync is indeed getting invoked, its just the current
> patch doesn't include the code block that invokes it. The function that
> invokes omap2_iommu_enable does so after a call to the
> pm_runtime_get_sync() call.

OK 

> > I think you are missing a layer here and it's the Linux kernel side
> > device driver for DSP that initializes things.
> 
> We already have separate drivers for MMUs (omap-iommu) and the processor
> management (omap-rproc). The former manages all the low-level
> programming sequences for the MMUs, while the latter manages the
> low-level reset etc, and is a client user of the respective IOMMU
> device. Both integrate into the respective frameworks. The IOMMU API
> invocations are handled in the remoteproc core, with the OMAP remoteproc
> driver publishing itself as having an MMU with the remoteproc core. The
> IOMMU API invoke the appropriate iommu_ops.
> 
> You can lookup the functions rproc_enable_iommu()/rproc_disable_iommu()
> in the remoteproc core. The IOMMU enabling sequences happen within the
> iommu_attach_device() API. The call flow is
> iommu_attach_device()->omap_iommu_attach_dev()->omap_iommu_attach()->
> iommu_enable()->
>omap_device_deassert_hardreset, pm_runtime_get_sync, omap2_iommu_enable.

OK. The thing to check here is that you have a separate device driver
for each sysc/syss device registers. This is because each hardware module
can be independently clocked and idled. Otherwise things will break at
some point. And no "things are configured for autoidle" or "we're not
doing PM is not a good excuse here" :)

Can you please check that? If the remoteproc driver and iommu driver
are tinkering with registers in separate hardware modules, we have
a layering violation. My guess is that we have at least two hardware
modules involved here, one for the iommu and one for the DSP device.
 
> >>> Typically we handle these registers by mapping them to the PM runtime
> >>> functions for the interconnect so we can reset and idle the hardware
> >>> modules even if there is no driver, see for example
> >>> omap54xx_mmu_dsp_hwmod.
> >>
> >> I haven't yet submitted the DRA7xx hwmods, but they will look almost
> >> exactly like the OMAP5 ones. The reset and idle on these are in general
> >> not effective at boot time since these are also controlled by a
> >> hard-reset line, so that's left to the drivers to deal with it through
> >> the omap_device_deassert_hardreset API.
> > 
> > If the MMU configuration is one time init, it may make sense to add
> > it to the hwmod reset function. However, if the Linux kernel side
> > driver needs to configure things depending on the DSP firmware, it
> > should be done in the kernel side device driver.
> 
> The MMU configuration comes into picture whenever the remoteproc driver
> is being booted and shut down, and also during suspend (no support for
> this yet in mainline on OMAP rproc). Today, the hwmod
> _enable/_idle/reset functions alone are not enough to power on the OMAP
> r

Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx

2015-07-23 Thread Tony Lindgren
* Suman Anna  [150723 09:25]:
> Hi Tony,
> 
> On 07/23/2015 02:24 AM, Tony Lindgren wrote:
> > * Suman Anna  [150722 09:25]:
> >> On 07/22/2015 12:26 AM, Tony Lindgren wrote:
> >>>
> >>> I don't like using syscon for tinkering directly with SoC registers.
> >>
> >> This is not a SoC-level register, but a register within a sub-module of
> >> the DSP processor sub-system. The DSP_SYSTEM sub-module in general is
> >> described in Section 5.3.3 of the TRM [1], and it implements different
> >> functionalities like the PRCM handshaking, wakeup logic and DSP
> >> subsystem top-level configuration. It is a module present within the DSP
> >> processor sub-system, so can only be accessed when the sub-system is
> >> clocked and the appropriate reset is released.
> > 
> > OK so if it's specific to the DSP driver along the lines of sysc and
> > syss registers.
> 
> There will be those registers too within the MMU config register space,
> even for DRA7xx MMUs. This is different, think of it like a register in
> the Control module except that it is present within the DSP sub-system
> instead of at the SoC level.

And what is taking care of pm_runtime_get here to ensure the module
is powered and clocked?

I think you are missing a layer here and it's the Linux kernel side
device driver for DSP that initializes things.
 
> > Typically we handle these registers by mapping them to the PM runtime
> > functions for the interconnect so we can reset and idle the hardware
> > modules even if there is no driver, see for example
> > omap54xx_mmu_dsp_hwmod.
> 
> I haven't yet submitted the DRA7xx hwmods, but they will look almost
> exactly like the OMAP5 ones. The reset and idle on these are in general
> not effective at boot time since these are also controlled by a
> hard-reset line, so that's left to the drivers to deal with it through
> the omap_device_deassert_hardreset API.

If the MMU configuration is one time init, it may make sense to add
it to the hwmod reset function. However, if the Linux kernel side
driver needs to configure things depending on the DSP firmware, it
should be done in the kernel side device driver. 
 
> >>> We should use some Linux generic framework for configuring these
> >>> bits to avoid nasty dependencies between various hardware modules
> >>> on the SoC.
> >>>
> >>> What does DSP_SYS_MMU_CONFIG register do? It seems it's probably
> >>> a regulator or a gate clock? If so, it should be set up as a
> >>> regulator or a clock and then the omap-iommu driver can just
> >>> use regulator or clcok framework to request the resource.
> >>
> >> No, its neither. It is a control bit that dictates whether the
> >> processor/EDMA addresses go through the respective MMU or not. The
> >> register currently has 4 bits (bit 0 in each nibble), one each for
> >> enabling each MMU and requesting an MMU abort on each. The MMU
> >> integration and enablement notes are detailed in Section 5.3.6 of the
> >> TRM [1], and the DSP_SYS_MMU_CONFIG register layout is in Table 5-28
> >> (Page 1641).
> > 
> > OK yeah seems like it should be handled by the DSP driver during
> > probe after doing pm_runtime_get. Then the driver can configure
> > things like IOMMU and load firmware. Or am I missing something here?
> 
> The DSP (remoteproc) driver uses the generic IOMMU API, and all the
> IOMMU enabling sequence is transparent to the DSP driver. So, any
> configuration/enabling sequence specific to the IOMMU has to be handled
> within the respective IOMMU platform driver that gets integrated into
> the IOMMU API.

To me it seems you still need a DSP kernel driver to manage the
DSP hardware for PM runtime. I don't see how iommu and remoteproc
alone would be sufficient here. You end up sprinkling DSP driver
specific code to the iommu and remoteproc layers.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx

2015-07-23 Thread Tony Lindgren
* Suman Anna  [150722 09:25]:
> On 07/22/2015 12:26 AM, Tony Lindgren wrote:
> > 
> > I don't like using syscon for tinkering directly with SoC registers.
> 
> This is not a SoC-level register, but a register within a sub-module of
> the DSP processor sub-system. The DSP_SYSTEM sub-module in general is
> described in Section 5.3.3 of the TRM [1], and it implements different
> functionalities like the PRCM handshaking, wakeup logic and DSP
> subsystem top-level configuration. It is a module present within the DSP
> processor sub-system, so can only be accessed when the sub-system is
> clocked and the appropriate reset is released.

OK so if it's specific to the DSP driver along the lines of sysc and
syss registers.

Typically we handle these registers by mapping them to the PM runtime
functions for the interconnect so we can reset and idle the hardware
modules even if there is no driver, see for example
omap54xx_mmu_dsp_hwmod.
 
> > We should use some Linux generic framework for configuring these
> > bits to avoid nasty dependencies between various hardware modules
> > on the SoC.
> > 
> > What does DSP_SYS_MMU_CONFIG register do? It seems it's probably
> > a regulator or a gate clock? If so, it should be set up as a
> > regulator or a clock and then the omap-iommu driver can just
> > use regulator or clcok framework to request the resource.
> 
> No, its neither. It is a control bit that dictates whether the
> processor/EDMA addresses go through the respective MMU or not. The
> register currently has 4 bits (bit 0 in each nibble), one each for
> enabling each MMU and requesting an MMU abort on each. The MMU
> integration and enablement notes are detailed in Section 5.3.6 of the
> TRM [1], and the DSP_SYS_MMU_CONFIG register layout is in Table 5-28
> (Page 1641).

OK yeah seems like it should be handled by the DSP driver during
probe after doing pm_runtime_get. Then the driver can configure
things like IOMMU and load firmware. Or am I missing something here?

Regards,

Tony

 
> [1] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx

2015-07-21 Thread Tony Lindgren
* Suman Anna  [150721 16:58]:
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -26,6 +26,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #include 
>  
> @@ -112,6 +114,18 @@ void omap_iommu_restore_ctx(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);
>  
> +static void dra7_cfg_dspsys_mmu(struct omap_iommu *obj, bool enable)
> +{
> + u32 val, mask;
> +
> + if (!obj->syscfg)
> + return;
> +
> + mask = (1 << (obj->id * DSP_SYS_MMU_CONFIG_EN_SHIFT));
> + val = enable ? mask : 0;
> + regmap_update_bits(obj->syscfg, DSP_SYS_MMU_CONFIG, mask, val);
> +}
> +
>  static void __iommu_set_twl(struct omap_iommu *obj, bool on)

I don't like using syscon for tinkering directly with SoC registers.

We should use some Linux generic framework for configuring these
bits to avoid nasty dependencies between various hardware modules
on the SoC.

What does DSP_SYS_MMU_CONFIG register do? It seems it's probably
a regulator or a gate clock? If so, it should be set up as a
regulator or a clock and then the omap-iommu driver can just
use regulator or clcok framework to request the resource.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 0/3] iommu: Remove OMAP IOVMM driver

2014-07-23 Thread Tony Lindgren
* Laurent Pinchart  [140723 07:02]:
> Hi Joerg,
> 
> On Wednesday 23 July 2014 15:52:17 Joerg Roedel wrote:
> > On Mon, Jul 21, 2014 at 11:19:29PM -0700, Tony Lindgren wrote:
> > > > Tony, is there still time to get this (and especially patch 2/3, which
> > > > touches arch/ code) in v3.17 ?
> > > 
> > > Yes as long as Joerg is OK to merge that branch in :)
> > 
> > Fine with me, I can take only patch 1 or all 3 into my arm/omap branch,
> > given Tony's Acked-by.
> > 
> > Then you guys can merge in this branch wherever you want :)
> 
> Thank you. Assuming there's currently no conflict to be resolved, I believe 
> the easiest would be for both you and Tony to merge my branch in your trees.

Yes please merge that branch via the IOMMU tree, I only need to merge it
if I see merge conflicts with it.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 0/3] iommu: Remove OMAP IOVMM driver

2014-07-21 Thread Tony Lindgren
* Laurent Pinchart  [140721 11:17]:
> Hi Tony and Joerg,
> 
> On Monday 21 July 2014 02:33:36 Tony Lindgren wrote:
> > * Laurent Pinchart  [140721 02:16]:
> > > Hi Suman, Joerg and Tony,
> > > 
> > > On Friday 18 July 2014 11:53:56 Suman Anna wrote:
> > > > On 07/18/2014 05:49 AM, Laurent Pinchart wrote:
> > > > > Hello,
> > > > > 
> > > > > The OMAP3 ISP driver was the only user of the OMAP IOVMM API. Now that
> > > > > is has been ported to the DMA API, remove the unused virtual memory
> > > > > manager.
> > > > > 
> > > > > The removal is split in three patches to ease upstream merge. The
> > > > > first patch removes the omap-iovmm driver, the second patch removes
> > > > > setting of now unused platform data fields from arch code, and the
> > > > > last patch cleans up the platform data structure.
> > > > 
> > > > Thanks for the revised series, it looks good. I have also tested the
> > > > series on OMAP3, OMAP4 and OMAP5.
> > > > 
> > > > For the changes in the entire series,
> > > > Acked-by: Suman Anna 
> > > 
> > > Thank you.
> > > 
> > > > > I'd like to get at least the first patch merged in v3.17. To avoid
> > > > > splitting the series across three kernel versions, it would be nice to
> > > > > also merge at least the second patch for v3.17. If there's no risk of
> > > > > conflict everything could be merged in one go through the ARM SoC
> > > > > tree. Otherwise a stable branch with patch 1/3 will be needed to base
> > > > > the arch change on.
> > > > > 
> > > > > Joerg, Tony, how would you like to proceed ?
> > > 
> > > The v3.17 merge window is getting close, it's probably too late to merge
> > > patch 2/3. Joerg, could you please take 1/3 in your tree for v3.17 ? 2/3
> > > and 3/3 would then get in v3.18. Tony, how would you like to proceed for
> > > those ?
> >
> > How about Joerg maybe do an immutable branch against v3.16-rc1
> > with just these three patches and merge it into your tree?
> > 
> > That way I too can merge the minimal branch in if there are conflics.
> > If that works for Joerg, then for arch/arm/*omap* changes:
> > 
> > Acked-by: Tony Lindgren 
> 
> I've created an immutable branch (or, rather, immutable until the changes 
> reach mainline, at which point I will remove the branch) on top of v3.16-rc1 
> with just the three patches from this series. You can find it at.
> 
>   git://linuxtv.org/pinchartl/media.git omap/iommu
> 
> Tony, is there still time to get this (and especially patch 2/3, which 
> touches 
> arch/ code) in v3.17 ?

Yes as long as Joerg is OK to merge that branch in :)

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 0/3] iommu: Remove OMAP IOVMM driver

2014-07-21 Thread Tony Lindgren
* Laurent Pinchart  [140721 02:16]:
> Hi Suman, Joerg and Tony,
> 
> On Friday 18 July 2014 11:53:56 Suman Anna wrote:
> > On 07/18/2014 05:49 AM, Laurent Pinchart wrote:
> > > Hello,
> > > 
> > > The OMAP3 ISP driver was the only user of the OMAP IOVMM API. Now that is
> > > has been ported to the DMA API, remove the unused virtual memory manager.
> > > 
> > > The removal is split in three patches to ease upstream merge. The first
> > > patch removes the omap-iovmm driver, the second patch removes setting of
> > > now unused platform data fields from arch code, and the last patch cleans
> > > up the platform data structure.
> > 
> > Thanks for the revised series, it looks good. I have also tested the
> > series on OMAP3, OMAP4 and OMAP5.
> > 
> > For the changes in the entire series,
> > Acked-by: Suman Anna 
> 
> Thank you.
> 
> > > I'd like to get at least the first patch merged in v3.17. To avoid
> > > splitting the series across three kernel versions, it would be nice to
> > > also merge at least the second patch for v3.17. If there's no risk of
> > > conflict everything could be merged in one go through the ARM SoC tree.
> > > Otherwise a stable branch with patch 1/3 will be needed to base the arch
> > > change on.
> > > 
> > > Joerg, Tony, how would you like to proceed ?
> 
> The v3.17 merge window is getting close, it's probably too late to merge 
> patch 
> 2/3. Joerg, could you please take 1/3 in your tree for v3.17 ? 2/3 and 3/3 
> would then get in v3.18. Tony, how would you like to proceed for those ?

How about Joerg maybe do an immutable branch against v3.16-rc1
with just these three patches and merge it into your tree?

That way I too can merge the minimal branch in if there are conflics.
If that works for Joerg, then for arch/arm/*omap* changes:

Acked-by: Tony Lindgren 

If the above is too complicated, then how about Laurent resend patches
2 - 3 once the dependencies have cleared so I can pick them up. This
is assuming nothing breaks with patchs 2 - 3 missing naturally :)

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 00/10] arch/arm OMAP IOMMU patches for 3.15

2014-03-12 Thread Tony Lindgren
* Suman Anna  [140312 10:25]:
> On 03/12/2014 12:04 PM, Tony Lindgren wrote:
> >
> >For the pdata-quirks.c changes, I assume you are working on
> >implementing the reset driver mentioned in the related patch?
> >
> 
> Dan Murphy is currently working on this, and there is still some
> work to be done before it can be posted to the lists.

OK thanks for the update, good to hear.

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 00/10] arch/arm OMAP IOMMU patches for 3.15

2014-03-12 Thread Tony Lindgren
* Suman Anna  [140311 14:52]:
> Hi Paul,
> 
> On 03/05/2014 06:24 PM, Suman Anna wrote:
> >Hi Paul, Benoit,
> >
> >This is a repost as per Tony's request [3] of all the OMAP IOMMU DT support
> >patches that are under arch/arm. The series just assimilates patches 8
> >through 13 from the v3 OMAP IOMMU DT adaptation for 3.15 series [1], and
> >all the v2 patches from the OMAP IOMMU DTS nodes [2].
> >
> >The only change made with respect to the patches above is in the OMAP4
> >and OMAP5 DTS nodes - I have adjusted the reg sizes from 0xff to 0x100.
> >
> >Can you please provide your acks on the hwmod patches and DTS
> >patches?
> 
> Ping. Can you review and provide your acks for Patches 2 and 5 (the
> hwmod patches) for Tony to pick up all the patches in the series?

Applied all with Paul's acks into omap-for-v3.15/dt. As it's getting
so late to the merge window, no guarantees it will actually get
pulled.

For the pdata-quirks.c changes, I assume you are working on
implementing the reset driver mentioned in the related patch?

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv3 00/13] OMAP IOMMU DT adaptation for 3.15

2014-03-05 Thread Tony Lindgren
* Suman Anna  [140304 09:03]:
> On 03/04/2014 10:04 AM, Joerg Roedel wrote:
> >
> >Applied patches 1-7 to my arm/omap branch.
> >
> >Tony, you can pull that branch into your tree if needed (when I pushed
> >it, which will happen today or tomorrow).

OK thanks, looks like remaining patches compile just fine even without
it which is nice.

> Tony,
> Can you also pull in the corresponding DTS node patches as well that
> go along with this series.
> http://marc.info/?l=linux-omap&m=139362062805816&w=2

They look OK to me, but looks like Benoit and Paul are not Cc:ed
on the hwmod changes. I suggest you resend just patches 8 - 13 and
the .dts changes as a new series with Benoit and Paul on Cc for
them so they have a chance to review and ack them.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv3 00/13] OMAP IOMMU DT adaptation for 3.15

2014-02-28 Thread Tony Lindgren
* Suman Anna  [140228 12:46]:
> Hi Joerg, Tony,
> 
> This is an updated series of the OMAP IOMMU DT adaptation intended
> for 3.15 merge window, addressing the comments from the v2 series.
> This series is rebased onto 3.14-rc4, and the only change to bindings
> is to drop the dma-window property.
> 
> The first 7 patches in the series are in drivers/iommu, with the first
> 3 patches performing some cleanup. The DT bindings and adaptation are
> done in patches 4 and 5.
> 
> Tony,
> Patches 8 through 13 are in arch/arm/mach-omap2 layer, so I am guessing
> these would have to go through your tree. Of these, patches 8 and 9 are
> cleanup fixes to get OMAP3 IVA MMU working, patches 10 & 11 are fixes
> required with DT-boot for OMAP3/4, patches 12 & 13 add the OMAP5 support.

Are patches 8 to 13 OK to apply separately from the iommu patches or
do they need to wait for the iommu patches to get merged first?

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs

2014-02-27 Thread Tony Lindgren
* Florian Vaussard  [140227 01:19]:
> Hi,
> 
> On 02/26/2014 06:59 PM, Suman Anna wrote:
> > Tony,
> > 
> > On 02/26/2014 11:18 AM, Tony Lindgren wrote:
> >> * Suman Anna  [140213 10:19]:
> >>> From: Florian Vaussard 
> >>>
> >>> The irq numbers, ocp address space and device attribute data
> >>> have all been cleaned up for OMAP3 IOMMUs. All this data is
> >>> populated via the corresponding dt node.
> >>>
> >>> Signed-off-by: Florian Vaussard 
> >>> Signed-off-by: Suman Anna 
> >>
> >> This will need to wait until we've made omap3 to be DT only
> >> as this will break idling of things for the legacy booting.
> >>
> > 
> > OK, will drop this and I will adjust Patch 9 to support the legacy boot
> > for OMAP3 ISP.
> > 
> 
> BTW, Tony do you have a new window for omap3 to be DT onyl?

I think a good point would be when we have DSS working with DT,
and have a .dts file for the Pandora board.

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv2 04/16] iommu/omap: add devicetree support

2014-02-26 Thread Tony Lindgren
* Suman Anna  [140213 10:19]:
> From: Florian Vaussard 
> 
> As OMAP2+ is moving to a full DT boot for all SoC families, commit
> 7ce93f3 "ARM: OMAP2+: Fix more missing data for omap3.dtsi file"
> adds basic DT bits for OMAP3. But the driver is not yet converted,
> so this will not work and driver will not be probed. Convert it!
> 
> Signed-off-by: Florian Vaussard 
> [s-a...@ti.com: dev_name adaptation and improved error checking]
> Signed-off-by: Suman Anna 

Best that this gets merged along with the other iommu patches, so
for the arch/arm/*omap* parts:

Acked-by: Tony Lindgren 

> ---
>  arch/arm/mach-omap2/omap-iommu.c |  5 +
>  drivers/iommu/omap-iommu.c   | 41 
> 
>  2 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap-iommu.c 
> b/arch/arm/mach-omap2/omap-iommu.c
> index f6daae8..f1fab56 100644
> --- a/arch/arm/mach-omap2/omap-iommu.c
> +++ b/arch/arm/mach-omap2/omap-iommu.c
> @@ -10,6 +10,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -58,6 +59,10 @@ static int __init omap_iommu_dev_init(struct omap_hwmod 
> *oh, void *unused)
>  
>  static int __init omap_iommu_init(void)
>  {
> + /* If dtb is there, the devices will be created dynamically */
> + if (of_have_populated_dt())
> + return -ENODEV;
> +
>   return omap_hwmod_for_each_by_class("mmu", omap_iommu_dev_init, NULL);
>  }
>  /* must be ready before omap3isp is probed */
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 6272c36..4329ab1 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -23,6 +23,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  
>  #include 
>  
> @@ -937,20 +940,41 @@ static int omap_iommu_probe(struct platform_device 
> *pdev)
>  {
>   int err = -ENODEV;
>   int irq;
> + size_t len;
>   struct omap_iommu *obj;
>   struct resource *res;
>   struct iommu_platform_data *pdata = pdev->dev.platform_data;
> + struct device_node *of = pdev->dev.of_node;
>  
>   obj = devm_kzalloc(&pdev->dev, sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
>   if (!obj)
>   return -ENOMEM;
>  
> - obj->nr_tlb_entries = pdata->nr_tlb_entries;
> - obj->name = pdata->name;
> + if (of) {
> + obj->name = dev_name(&pdev->dev);
> + obj->nr_tlb_entries = 32;
> + err = of_property_read_u32(of, "ti,#tlb-entries",
> +&obj->nr_tlb_entries);
> + if (err && err != -EINVAL)
> + return err;
> + if (obj->nr_tlb_entries != 32 && obj->nr_tlb_entries != 8)
> + return -EINVAL;
> + err = of_get_dma_window(of, NULL, 0, NULL, &obj->da_start,
> + &len);
> + if (err != 0)
> + return err;
> + obj->da_end = obj->da_start + len;
> + } else {
> + obj->nr_tlb_entries = pdata->nr_tlb_entries;
> + obj->name = pdata->name;
> + obj->da_start = pdata->da_start;
> + obj->da_end = pdata->da_end;
> + }
> + if (obj->da_end <= obj->da_start)
> + return -EINVAL;
> +
>   obj->dev = &pdev->dev;
>   obj->ctx = (void *)obj + sizeof(*obj);
> - obj->da_start = pdata->da_start;
> - obj->da_end = pdata->da_end;
>  
>   spin_lock_init(&obj->iommu_lock);
>   mutex_init(&obj->mmap_lock);
> @@ -991,11 +1015,20 @@ static int omap_iommu_remove(struct platform_device 
> *pdev)
>   return 0;
>  }
>  
> +static struct of_device_id omap_iommu_of_match[] = {
> + { .compatible = "ti,omap2-iommu" },
> + { .compatible = "ti,omap4-iommu" },
> + { .compatible = "ti,dra7-iommu" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, omap_iommu_of_match);
> +
>  static struct platform_driver omap_iommu_driver = {
>   .probe  = omap_iommu_probe,
>   .remove = omap_iommu_remove,
>   .driver = {
>   .name   = "omap-iommu",
> + .of_match_table = of_match_ptr(omap_iommu_of_match),
>   },
>  };
>  
> -- 
> 1.8.5.3
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv2 08/16] ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2

2014-02-26 Thread Tony Lindgren
* Laurent Pinchart  [140225 13:18]:
> On Thursday 13 February 2014 12:15:39 Suman Anna wrote:
> > From: Florian Vaussard 
> > 
> > CONFIG_OMAP_IOMMU_IVA2 was defined originally to avoid conflicting
> > usage by tidspbridge and other iommu users. The same can be achieved
> > by marking the DT node disabled, so remove this obsolete flag and
> > the corresponding hwmod data can be enabled.
> > 
> > Cc: Paul Walmsley 
> > Signed-off-by: Florian Vaussard 
> > [s-a...@ti.com: revise commit log]
> > Signed-off-by: Suman Anna 
> 
> Acked-by: Laurent Pinchart 

Acked-by: Tony Lindgren 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs

2014-02-26 Thread Tony Lindgren
* Suman Anna  [140213 10:19]:
> From: Florian Vaussard 
> 
> The irq numbers, ocp address space and device attribute data
> have all been cleaned up for OMAP3 IOMMUs. All this data is
> populated via the corresponding dt node.
> 
> Signed-off-by: Florian Vaussard 
> Signed-off-by: Suman Anna 

This will need to wait until we've made omap3 to be DT only
as this will break idling of things for the legacy booting.

Regards,

Tony

> ---
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 46 
> --
>  1 file changed, 46 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> index 9c7e23a..d68c131 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -24,7 +24,6 @@
>  #include "l4_3xxx.h"
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  
> @@ -2991,83 +2990,39 @@ static struct omap_hwmod_class 
> omap3xxx_mmu_hwmod_class = {
>  
>  /* mmu isp */
>  
> -static struct omap_mmu_dev_attr mmu_isp_dev_attr = {
> - .da_start   = 0x0,
> - .da_end = 0xf000,
> - .nr_tlb_entries = 8,
> -};
> -
>  static struct omap_hwmod omap3xxx_mmu_isp_hwmod;
> -static struct omap_hwmod_irq_info omap3xxx_mmu_isp_irqs[] = {
> - { .irq = 24 + OMAP_INTC_START, },
> - { .irq = -1 }
> -};
> -
> -static struct omap_hwmod_addr_space omap3xxx_mmu_isp_addrs[] = {
> - {
> - .pa_start   = 0x480bd400,
> - .pa_end = 0x480bd47f,
> - .flags  = ADDR_TYPE_RT,
> - },
> - { }
> -};
>  
>  /* l4_core -> mmu isp */
>  static struct omap_hwmod_ocp_if omap3xxx_l4_core__mmu_isp = {
>   .master = &omap3xxx_l4_core_hwmod,
>   .slave  = &omap3xxx_mmu_isp_hwmod,
> - .addr   = omap3xxx_mmu_isp_addrs,
>   .user   = OCP_USER_MPU | OCP_USER_SDMA,
>  };
>  
>  static struct omap_hwmod omap3xxx_mmu_isp_hwmod = {
>   .name   = "mmu_isp",
>   .class  = &omap3xxx_mmu_hwmod_class,
> - .mpu_irqs   = omap3xxx_mmu_isp_irqs,
>   .main_clk   = "cam_ick",
> - .dev_attr   = &mmu_isp_dev_attr,
>   .flags  = HWMOD_NO_IDLEST,
>  };
>  
>  /* mmu iva */
>  
> -static struct omap_mmu_dev_attr mmu_iva_dev_attr = {
> - .da_start   = 0x1100,
> - .da_end = 0xf000,
> - .nr_tlb_entries = 32,
> -};
> -
>  static struct omap_hwmod omap3xxx_mmu_iva_hwmod;
> -static struct omap_hwmod_irq_info omap3xxx_mmu_iva_irqs[] = {
> - { .irq = 28 + OMAP_INTC_START, },
> - { .irq = -1 }
> -};
> -
>  static struct omap_hwmod_rst_info omap3xxx_mmu_iva_resets[] = {
>   { .name = "mmu", .rst_shift = 1, .st_shift = 9 },
>  };
>  
> -static struct omap_hwmod_addr_space omap3xxx_mmu_iva_addrs[] = {
> - {
> - .pa_start   = 0x5d00,
> - .pa_end = 0x5d7f,
> - .flags  = ADDR_TYPE_RT,
> - },
> - { }
> -};
> -
>  /* l3_main -> iva mmu */
>  static struct omap_hwmod_ocp_if omap3xxx_l3_main__mmu_iva = {
>   .master = &omap3xxx_l3_main_hwmod,
>   .slave  = &omap3xxx_mmu_iva_hwmod,
> - .addr   = omap3xxx_mmu_iva_addrs,
>   .user   = OCP_USER_MPU | OCP_USER_SDMA,
>  };
>  
>  static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>   .name   = "mmu_iva",
>   .class  = &omap3xxx_mmu_hwmod_class,
> - .mpu_irqs   = omap3xxx_mmu_iva_irqs,
>   .clkdm_name = "iva2_clkdm",
>   .rst_lines  = omap3xxx_mmu_iva_resets,
>   .rst_lines_cnt  = ARRAY_SIZE(omap3xxx_mmu_iva_resets),
> @@ -3080,7 +3035,6 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>   .idlest_idle_bit = OMAP3430_ST_IVA2_SHIFT,
>   },
>   },
> - .dev_attr   = &mmu_iva_dev_attr,
>   .flags  = HWMOD_NO_IDLEST,
>  };
>  
> -- 
> 1.8.5.3
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv2 10/16] ARM: OMAP2+: use pdata quirks for iommu reset lines

2014-02-26 Thread Tony Lindgren
* Suman Anna  [140213 10:19]:
> The OMAP iommu driver performs the reset management for the
> iommu instances in processor sub-systems using the omap_device
> API which are currently supplied as platform data ops. Use pdata
> quirks to maintain the functionality as the OMAP iommu driver
> gets converted to use DT nodes, until the reset portions are
> decoupled from omap_hwmod/omap_device into a separate reset
> driver.
> 
> This patch adds the pdata quirks for the reset management of
> iommus within the DSP (OMAP3 & OMAP4) and IPU subsystems (OMAP4).
> 
> Signed-off-by: Suman Anna 

Looks OK, but I suggest you separate out the remaining patches in
this series into another clean-up series. Then the clean-up series
can be merged later on as these have a good chance of conflicting
with other stuff.

Tony

> ---
>  arch/arm/mach-omap2/pdata-quirks.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/pdata-quirks.c 
> b/arch/arm/mach-omap2/pdata-quirks.c
> index 3d5b24d..74e094a 100644
> --- a/arch/arm/mach-omap2/pdata-quirks.c
> +++ b/arch/arm/mach-omap2/pdata-quirks.c
> @@ -16,12 +16,14 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  #include "am35xx.h"
>  #include "common.h"
>  #include "common-board-devices.h"
>  #include "dss-common.h"
>  #include "control.h"
> +#include "omap_device.h"
>  
>  struct pdata_init {
>   const char *compatible;
> @@ -92,6 +94,12 @@ static void __init hsmmc2_internal_input_clk(void)
>   omap_ctrl_writel(reg, OMAP343X_CONTROL_DEVCONF1);
>  }
>  
> +static struct iommu_platform_data omap3_iommu_pdata = {
> + .reset_name = "mmu",
> + .assert_reset = omap_device_assert_hardreset,
> + .deassert_reset = omap_device_deassert_hardreset,
> +};
> +
>  static int omap3_sbc_t3730_twl_callback(struct device *dev,
>  unsigned gpio,
>  unsigned ngpio)
> @@ -185,6 +193,12 @@ static void __init omap4_panda_legacy_init(void)
>   legacy_init_ehci_clk("auxclk3_ck");
>   legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
>  }
> +
> +static struct iommu_platform_data omap4_iommu_pdata = {
> + .reset_name = "mmu_cache",
> + .assert_reset = omap_device_assert_hardreset,
> + .deassert_reset = omap_device_deassert_hardreset,
> +};
>  #endif
>  
>  #ifdef CONFIG_SOC_OMAP5
> @@ -240,6 +254,8 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
>  #ifdef CONFIG_ARCH_OMAP3
>   OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002030, "48002030.pinmux", 
> &pcs_pdata),
>   OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002a00, "48002a00.pinmux", 
> &pcs_pdata),
> + OF_DEV_AUXDATA("ti,omap2-iommu", 0x5d00, "5d00.mmu",
> +&omap3_iommu_pdata),
>   /* Only on am3517 */
>   OF_DEV_AUXDATA("ti,davinci_mdio", 0x5c03, "davinci_mdio.0", NULL),
>   OF_DEV_AUXDATA("ti,am3517-emac", 0x5c00, "davinci_emac.0",
> @@ -248,6 +264,10 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = 
> {
>  #ifdef CONFIG_ARCH_OMAP4
>   OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a100040, "4a100040.pinmux", 
> &pcs_pdata),
>   OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a31e040, "4a31e040.pinmux", 
> &pcs_pdata),
> + OF_DEV_AUXDATA("ti,omap4-iommu", 0x4a066000, "4a066000.mmu",
> +&omap4_iommu_pdata),
> + OF_DEV_AUXDATA("ti,omap4-iommu", 0x55082000, "55082000.mmu",
> +&omap4_iommu_pdata),
>  #endif
>   { /* sentinel */ },
>  };
> -- 
> 1.8.5.3
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCHv2 08/16] ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2

2014-02-26 Thread Tony Lindgren
* Laurent Pinchart  [140225 13:18]:
> On Thursday 13 February 2014 12:15:39 Suman Anna wrote:
> > From: Florian Vaussard 
> > 
> > CONFIG_OMAP_IOMMU_IVA2 was defined originally to avoid conflicting
> > usage by tidspbridge and other iommu users. The same can be achieved
> > by marking the DT node disabled, so remove this obsolete flag and
> > the corresponding hwmod data can be enabled.
> > 
> > Cc: Paul Walmsley 
> > Signed-off-by: Florian Vaussard 
> > [s-a...@ti.com: revise commit log]
> > Signed-off-by: Suman Anna 
> 
> Acked-by: Laurent Pinchart 

Acked-by: Tony Lindgren 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 8/9] iommu: OMAP: build only on OMAP2+

2013-03-05 Thread Tony Lindgren
* Arnd Bergmann  [130305 14:21]:
> The OMAP IOMMU driver intentionally fails to build on OMAP1
> platforms, so we should not allow enabling it there.
> 
> Signed-off-by: Arnd Bergmann 
> Cc: Joerg Roedel 
> Cc: iommu@lists.linux-foundation.org
> Cc: Ohad Ben-Cohen 
> Cc: Tony Lindgren 
> Cc: Omar Ramirez Luna 

Makes sense, I doubt that anybody will work on omap1 version
at this point:

Acked-by: Tony Lindgren 

> ---
>  drivers/iommu/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 5c514d07..c332fb9 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -130,7 +130,7 @@ config IRQ_REMAP
>  # OMAP IOMMU support
>  config OMAP_IOMMU
>   bool "OMAP IOMMU Support"
> - depends on ARCH_OMAP
> + depends on ARCH_OMAP2PLUS
>   select IOMMU_API
>  
>  config OMAP_IOVMM
> -- 
> 1.8.1.2
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 5/5] ARM: OMAP4: hwmod data: ipu and dsp to use parent clocks instead of leaf clocks

2012-11-21 Thread Tony Lindgren
* Omar Ramirez Luna  [121119 17:08]:
> This prevents hwmod _enable_clocks...omap2_dflt_clk_enable path
> from enabling modulemode inside CLKCTRL using its clk->enable_reg
> field. Instead is left to _omap4_enable_module though soc_ops, as
> the one in charge of this setting.
> 
> According to comments received[1] for related patches the idea is
> to get rid of leaf clocks in future. So remove these two while at it.
> 
> [1] http://lkml.org/lkml/2012/8/20/226

This one should be queued by Paul, or at least acked by him.

Regards,

Tony

 
> Signed-off-by: Omar Ramirez Luna 
> ---
>  arch/arm/mach-omap2/clock44xx_data.c   |   22 --
>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |4 ++--
>  2 files changed, 2 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clock44xx_data.c 
> b/arch/arm/mach-omap2/clock44xx_data.c
> index 6efc30c..067c486 100644
> --- a/arch/arm/mach-omap2/clock44xx_data.c
> +++ b/arch/arm/mach-omap2/clock44xx_data.c
> @@ -1316,16 +1316,6 @@ static struct clk dmic_fck = {
>   .clkdm_name = "abe_clkdm",
>  };
>  
> -static struct clk dsp_fck = {
> - .name   = "dsp_fck",
> - .ops= &clkops_omap2_dflt,
> - .enable_reg = OMAP4430_CM_TESLA_TESLA_CLKCTRL,
> - .enable_bit = OMAP4430_MODULEMODE_HWCTRL,
> - .clkdm_name = "tesla_clkdm",
> - .parent = &dpll_iva_m4x2_ck,
> - .recalc = &followparent_recalc,
> -};
> -
>  static struct clk dss_sys_clk = {
>   .name   = "dss_sys_clk",
>   .ops= &clkops_omap2_dflt,
> @@ -1696,16 +1686,6 @@ static struct clk i2c4_fck = {
>   .recalc = &followparent_recalc,
>  };
>  
> -static struct clk ipu_fck = {
> - .name   = "ipu_fck",
> - .ops= &clkops_omap2_dflt,
> - .enable_reg = OMAP4430_CM_DUCATI_DUCATI_CLKCTRL,
> - .enable_bit = OMAP4430_MODULEMODE_HWCTRL,
> - .clkdm_name = "ducati_clkdm",
> - .parent = &ducati_clk_mux_ck,
> - .recalc = &followparent_recalc,
> -};
> -
>  static struct clk iss_ctrlclk = {
>   .name   = "iss_ctrlclk",
>   .ops= &clkops_omap2_dflt,
> @@ -3151,7 +3131,6 @@ static struct omap_clk omap44xx_clks[] = {
>   CLK(NULL,   "div_ts_ck",&div_ts_ck, 
> CK_446X),
>   CLK(NULL,   "dmic_sync_mux_ck", &dmic_sync_mux_ck,  
> CK_443X),
>   CLK(NULL,   "dmic_fck", &dmic_fck,  
> CK_443X),
> - CLK(NULL,   "dsp_fck",  &dsp_fck,   
> CK_443X),
>   CLK(NULL,   "dss_sys_clk",  &dss_sys_clk,   
> CK_443X),
>   CLK(NULL,   "dss_tv_clk",   &dss_tv_clk,
> CK_443X),
>   CLK(NULL,   "dss_48mhz_clk",&dss_48mhz_clk, 
> CK_443X),
> @@ -3183,7 +3162,6 @@ static struct omap_clk omap44xx_clks[] = {
>   CLK(NULL,   "i2c2_fck", &i2c2_fck,  
> CK_443X),
>   CLK(NULL,   "i2c3_fck", &i2c3_fck,  
> CK_443X),
>   CLK(NULL,   "i2c4_fck", &i2c4_fck,  
> CK_443X),
> - CLK(NULL,   "ipu_fck",  &ipu_fck,   
> CK_443X),
>   CLK(NULL,   "iss_ctrlclk",  &iss_ctrlclk,   
> CK_443X),
>   CLK(NULL,   "iss_fck",  &iss_fck,   
> CK_443X),
>   CLK(NULL,   "iva_fck",  &iva_fck,   
> CK_443X),
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c 
> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index aab5c12..1f61093 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -650,7 +650,7 @@ static struct omap_hwmod omap44xx_dsp_hwmod = {
>   .mpu_irqs   = omap44xx_dsp_irqs,
>   .rst_lines  = omap44xx_dsp_resets,
>   .rst_lines_cnt  = ARRAY_SIZE(omap44xx_dsp_resets),
> - .main_clk   = "dsp_fck",
> + .main_clk   = "dpll_iva_m4x2_ck",
>   .prcm = {
>   .omap4 = {
>   .clkctrl_offs = OMAP4_CM_TESLA_TESLA_CLKCTRL_OFFSET,
> @@ -1677,7 +1677,7 @@ static struct omap_hwmod omap44xx_ipu_hwmod = {
>   .mpu_irqs   = omap44xx_ipu_irqs,
>   .rst_lines  = omap44xx_ipu_resets,
>   .rst_lines_cnt  = ARRAY_SIZE(omap44xx_ipu_resets),
> - .main_clk   = "ipu_fck",
> + .main_clk   = "ducati_clk_mux_ck",
>   .prcm = {
>   .omap4 = {
>   .clkctrl_offs = OMAP4_CM_DUCATI_DUCATI_CLKCTRL_OFFSET,
> -- 
> 1.7.4.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 4/5] iommu/omap: adapt to runtime pm

2012-11-21 Thread Tony Lindgren
* Omar Ramirez Luna  [121119 17:08]:
> Use runtime PM functionality interfaced with hwmod enable/idle
> functions, to replace direct clock operations and sysconfig
> handling.
> 
> Due to reset sequence, pm_runtime_[get|put]_sync must be used, to
> avoid possible operations with the module under reset. Because of
> this and given that the driver uses spin_locks to protect their
> critical sections, we must use pm_runtime_irq_safe in order for the
> runtime ops to be happy, otherwise might_sleep_if checks in runtime
> framework will complain.
> 
> The remaining pm_runtime out of iommu_enable and iommu_disable
> corresponds to paths that can be accessed through debugfs, some of
> them doesn't work if the module is not enabled first, but in future
> if the mmu is idled withouth freeing, these are needed to debug.
> 
> Signed-off-by: Omar Ramirez Luna 
> ---
>  arch/arm/mach-omap2/omap-iommu.c |1 -
>  drivers/iommu/omap-iommu.c   |   40 ++---
>  drivers/iommu/omap-iommu.h   |3 --
>  drivers/iommu/omap-iommu2.c  |   17 
>  include/linux/platform_data/iommu-omap.h |1 -
>  5 files changed, 19 insertions(+), 43 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap-iommu.c 
> b/arch/arm/mach-omap2/omap-iommu.c
> index 02726a6..7642fc4 100644
> --- a/arch/arm/mach-omap2/omap-iommu.c
> +++ b/arch/arm/mach-omap2/omap-iommu.c
> @@ -31,7 +31,6 @@ static int __init omap_iommu_dev_init(struct omap_hwmod 
> *oh, void *unused)
>   return -ENOMEM;
>  
>   pdata->name = oh->name;
> - pdata->clk_name = oh->main_clk;
>   pdata->nr_tlb_entries = a->nr_tlb_entries;
>   pdata->da_start = a->da_start;
>   pdata->da_end = a->da_end;

The runtime PM related changes would be good to be checked
by Kevin, added him to cc. For the arch/arm/mach-omap2/ change above:

Acked-by: Tony Lindgren 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 0/6] OMAP: iommu: hwmod, reset handling and runtime PM

2012-10-18 Thread Tony Lindgren
Omar,

* Omar Ramirez Luna  [121017 16:54]:
> On 16 October 2012 12:22, Tony Lindgren  wrote:
>
> > These will need to be rebased on omap-for-v3.8/cleanup-headers-iommu
> > when I have that pushed out as that removes plat/*iommu*.h files.
> 
> Ok, will wait and rebase on top of it.

Thanks, the related patches are now posted in thread
"[PATCH v3 0/6] omap iommu changes to remove plat includes".

Also, can you please take a look at the "Updated status of the removal
of plat headers" thread?

I've tagged you to remove the omap plat/mailbox.h :) 

Regards,

Tony
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 0/6] OMAP: iommu: hwmod, reset handling and runtime PM

2012-10-16 Thread Tony Lindgren
* Omar Ramirez Luna  [121011 18:07]:
> These patches are needed for remoteproc to work on OMAP4.
> 
> Introduced iommu hwmod support for OMAP3 (iva, isp) and
> OMAP4 (ipu, dsp), along with the corresponding runtime PM
> and routines to deassert reset lines, enable/disable clocks
> and configure sysc registers.
> 
> Although IOMMU hwmod patches were already submitted in the past,
> this series adds few more changes, like:
> - New reset handling.
> - Save and restore context code rework.
> - Device tree bindings for OMAP3 and OMAP4.
> 
> For this series I just dropped the patches already included in
> mainline.

These will need to be rebased on omap-for-v3.8/cleanup-headers-iommu
when I have that pushed out as that removes plat/*iommu*.h files.

Regards,

Tony
 
> Previous work can be found at:
> [v2]
>   http://www.mail-archive.com/linux-omap@vger.kernel.org/msg75701.html
> [v1]
>   http://www.mail-archive.com/linux-omap@vger.kernel.org/msg70447.html
> 
> [old iteration without reset, save/restore and device tree]
>   http://www.mail-archive.com/linux-omap@vger.kernel.org/msg60133.html
> 
> Omar Ramirez Luna (6):
>   ARM: OMAP3/4: iommu: migrate to hwmod framework
>   ARM: OMAP3/4: iommu: adapt to runtime pm
>   ARM: OMAP: iommu: pm runtime save and restore context
>   ARM: OMAP: iommu: optimize save and restore routines
>   ARM: OMAP: iommu: add device tree support
>   arm/dts: OMAP3/4: Add iommu nodes
> 
>  .../devicetree/bindings/arm/omap/iommu.txt |   10 ++
>  arch/arm/boot/dts/omap3.dtsi   |   12 +-
>  arch/arm/boot/dts/omap4.dtsi   |   17 +-
>  arch/arm/mach-omap2/devices.c  |2 +-
>  arch/arm/mach-omap2/iommu2.c   |   74 ++--
>  arch/arm/mach-omap2/omap-iommu.c   |  176 
> +---
>  arch/arm/plat-omap/include/plat/iommu.h|   20 ++-
>  arch/arm/plat-omap/include/plat/iommu2.h   |4 -
>  drivers/iommu/omap-iommu.c |  163 ++
>  9 files changed, 245 insertions(+), 233 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/iommu.txt
> 
> -- 
> 1.7.9.5
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu