Hi Greg,
The situation of tidspbridge driver on staging has been pretty sad,
basically, it has never worked. This is a step backwards from the
previous situation where it was clear which branch to use to get it
working. Unfortunately, the ARM and OMAP trees haven't made it easy,
as changes in those trees have broken it even further.
My proposal to get this fixed is:
1) Revert back the iommu changes
Presumably, in order to get the migration to omap iommu working many
dependencies are needed, which haven't been merged yet. And at least I
have never seen it working, although Fernando claims to have the right
set of patches to do so. I say merging this was premature.
I have reverted back those changes in:
git://gitorious.org/~felipec/linux-omap/felipec.git fc-dsp-iommu-revert-v37-rc1
That would have been enough for 2.6.36, but...
2) Apply the fixes for recent omap control changes
On 2.6.37-rc1, omap platform internals changed, so the build is broken
again. Paul Walmsley provided some reference patches that I have
modified slightly.
3) Fix for memblock
Changes in memblock also broke this driver, one patch of mine should
fix that, as well as prepare it for imminent changes from the ARM
tree.
All the changes are in:
git://gitorious.org/~felipec/linux-omap/felipec.git fc-dsp-fix-v37-rc1
With this, the driver should work on 2.6.37, and would be the first
time it does so on staging.
I didn't see any point in sending the patches to review the reverts,
I'm going to send the rest of the patches on top of the branch
fc-dsp-iommu-revert-v37-rc1.
These are the changes since v2.6.37-rc1:
Felipe Contreras (13):
Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
Revert "staging: tidspbridge - remove dmm custom module"
Revert "staging: tidspbridge - deprecate
reserve/unreserve_memory funtions"
Revert "staging: tidspbridge - remove reserved memory clean up"
Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
Revert "staging: tidspbridge - move all iommu related code to a new file"
Revert "staging: tidspbridge - remove hw directory"
Revert "staging: tidspbridge - fix mmufault support"
Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap
to a proper name"
Revert "staging: tidspbridge - move shared memory iommu maps to
tiomap3430.c"
Revert "staging: tidspbridge: replace iommu custom for
opensource implementation"
omap: dsp: remove shm from normal memory
Paul Walmsley (4):
omap: control: add functions for DSP boot
omap: pm: use control functions in DSP reset code
omap: dsp: add boot control functions
staging: tidspbridge: use boot control functions
Cheers.
--
Felipe Contreras
On Sun, Nov 7, 2010 at 11:36 PM, Felipe Contreras
<[email protected]> wrote:
> The situation of tidspbridge driver on staging has been pretty sad,
> basically, it has never worked. This is a step backwards from the
> previous situation where it was clear which branch to use to get it
> working. Unfortunately, the ARM and OMAP trees haven't made it easy,
> as changes in those trees have broken it even further.
Here's another try, this one has only one patch that needs omap
maintainers to ack.
git://gitorious.org/~felipec/linux-omap/felipec.git fc-dsp-fix-v37-rc1-min
These are the changes since v2.6.37-rc1:
Felipe Contreras (14):
Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
Revert "staging: tidspbridge - remove dmm custom module"
Revert "staging: tidspbridge - deprecate
reserve/unreserve_memory funtions"
Revert "staging: tidspbridge - remove reserved memory clean up"
Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
Revert "staging: tidspbridge - move all iommu related code to a new file"
Revert "staging: tidspbridge - remove hw directory"
Revert "staging: tidspbridge - fix mmufault support"
Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap
to a proper name"
Revert "staging: tidspbridge - move shared memory iommu maps to
tiomap3430.c"
Revert "staging: tidspbridge: replace iommu custom for
opensource implementation"
staging: tidspbridge: hack to make it compile on v37-rc1
omap: dsp: remove shm from normal memory
--
Felipe Contreras
* Felipe Contreras <[email protected]> [101108 14:51]:
> On Sun, Nov 7, 2010 at 11:36 PM, Felipe Contreras
> <[email protected]> wrote:
> > The situation of tidspbridge driver on staging has been pretty sad,
> > basically, it has never worked. This is a step backwards from the
> > previous situation where it was clear which branch to use to get it
> > working. Unfortunately, the ARM and OMAP trees haven't made it easy,
> > as changes in those trees have broken it even further.
Hmm to me it looks like these reverts you are suggesting are
for drivers/staging/tidspbridge, not for arch/arm/* :)
> Here's another try, this one has only one patch that needs omap
> maintainers to ack.
Acked that memblock patch it in this thread.
Tony
On Tuesday 09 November 2010, Felipe Contreras wrote:
> Felipe Contreras (14):
> Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
> Revert "staging: tidspbridge - remove dmm custom module"
> Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
> Revert "staging: tidspbridge - remove reserved memory clean up"
> Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
> Revert "staging: tidspbridge - move all iommu related code to a new file"
> Revert "staging: tidspbridge - remove hw directory"
> Revert "staging: tidspbridge - fix mmufault support"
> Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
> Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
> Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
> Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
That adds quite a lot of crap back in that was removed by Fernando earlier:
44 files changed, 3733 insertions(+), 847 deletions(-)
It may have been premature to merge the patches as you say, but now that
they are in, I'd vote for giving Fernando a chance to fix up any damage
that was done in the process rather than just reverting all the useful
changes.
Arnd
On Tue, Nov 9, 2010 at 10:29 AM, Arnd Bergmann <[email protected]> wrote:
> On Tuesday 09 November 2010, Felipe Contreras wrote:
>> Felipe Contreras (14):
>> ? ? ? Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>> ? ? ? Revert "staging: tidspbridge - remove dmm custom module"
>> ? ? ? Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>> ? ? ? Revert "staging: tidspbridge - remove reserved memory clean up"
>> ? ? ? Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>> ? ? ? Revert "staging: tidspbridge - move all iommu related code to a new file"
>> ? ? ? Revert "staging: tidspbridge - remove hw directory"
>> ? ? ? Revert "staging: tidspbridge - fix mmufault support"
>> ? ? ? Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>> ? ? ? Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>> ? ? ? Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>> ? ? ? Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>
> That adds quite a lot of crap back in that was removed by Fernando earlier:
>
> ?44 files changed, 3733 insertions(+), 847 deletions(-)
>
> It may have been premature to merge the patches as you say, but now that
> they are in, I'd vote for giving Fernando a chance to fix up any damage
> that was done in the process rather than just reverting all the useful
> changes.
>
> ? ? ? ?Arnd
tidspbridge iommu change are working fine all the patches and few fixes after
that are alredy sent. what breaks tidspbridge, is the unmerged
dependencies in linux omap
tree, specifically the iommu module patches and the SG patch. if the
dependencies are merged tidspbridge work perfectly I don't need to fix
anything. As the dependencies are not merge yet it is ok to revert and
once push once all the dependencies are merge. I am not familiar with
that so I don't know how much time can it take.
Regards,
Fernando.
>
On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
> On Tuesday 09 November 2010, Felipe Contreras wrote:
> > Felipe Contreras (14):
> > Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
> > Revert "staging: tidspbridge - remove dmm custom module"
> > Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
> > Revert "staging: tidspbridge - remove reserved memory clean up"
> > Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
> > Revert "staging: tidspbridge - move all iommu related code to a new file"
> > Revert "staging: tidspbridge - remove hw directory"
> > Revert "staging: tidspbridge - fix mmufault support"
> > Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
> > Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
> > Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
> > Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>
> That adds quite a lot of crap back in that was removed by Fernando earlier:
>
> 44 files changed, 3733 insertions(+), 847 deletions(-)
>
> It may have been premature to merge the patches as you say, but now that
> they are in, I'd vote for giving Fernando a chance to fix up any damage
> that was done in the process rather than just reverting all the useful
> changes.
In looking at this further, I agree.
Felipe, are all of these really needing to be reverted? How about
picking out the functional changes that need to be resolved instead of
just rolling back everything that has been done here. Surely not all of
these are wrong, right?
thanks,
greg k-h
On Tue, Nov 9, 2010 at 10:55 AM, Greg KH <[email protected]> wrote:
> On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
>> On Tuesday 09 November 2010, Felipe Contreras wrote:
>> > Felipe Contreras (14):
>> > ? ? ? Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>> > ? ? ? Revert "staging: tidspbridge - remove dmm custom module"
>> > ? ? ? Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>> > ? ? ? Revert "staging: tidspbridge - remove reserved memory clean up"
>> > ? ? ? Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>> > ? ? ? Revert "staging: tidspbridge - move all iommu related code to a new file"
>> > ? ? ? Revert "staging: tidspbridge - remove hw directory"
>> > ? ? ? Revert "staging: tidspbridge - fix mmufault support"
>> > ? ? ? Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>> > ? ? ? Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>> > ? ? ? Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>> > ? ? ? Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>>
>> That adds quite a lot of crap back in that was removed by Fernando earlier:
>>
>> ?44 files changed, 3733 insertions(+), 847 deletions(-)
>>
>> It may have been premature to merge the patches as you say, but now that
>> they are in, I'd vote for giving Fernando a chance to fix up any damage
>> that was done in the process rather than just reverting all the useful
>> changes.
>
> In looking at this further, I agree.
>
> Felipe, are all of these really needing to be reverted? ?How about
> picking out the functional changes that need to be resolved instead of
> just rolling back everything that has been done here. ?Surely not all of
> these are wrong, right?
Patches are _NOT_ wrong, missing dependencies break the bridge.
Without that dependencies the first patch of the set won't work and
all other patches have dependency on the first one, so all of them
need to be reverted.
Regards,
Fernando.
>
> thanks,
>
> greg k-h
>
On Tue, Nov 09, 2010 at 11:04:18AM -0600, Guzman Lugo, Fernando wrote:
> On Tue, Nov 9, 2010 at 10:55 AM, Greg KH <[email protected]> wrote:
> > On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
> >> On Tuesday 09 November 2010, Felipe Contreras wrote:
> >> > Felipe Contreras (14):
> >> > ? ? ? Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
> >> > ? ? ? Revert "staging: tidspbridge - remove dmm custom module"
> >> > ? ? ? Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
> >> > ? ? ? Revert "staging: tidspbridge - remove reserved memory clean up"
> >> > ? ? ? Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
> >> > ? ? ? Revert "staging: tidspbridge - move all iommu related code to a new file"
> >> > ? ? ? Revert "staging: tidspbridge - remove hw directory"
> >> > ? ? ? Revert "staging: tidspbridge - fix mmufault support"
> >> > ? ? ? Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
> >> > ? ? ? Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
> >> > ? ? ? Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
> >> > ? ? ? Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
> >>
> >> That adds quite a lot of crap back in that was removed by Fernando earlier:
> >>
> >> ?44 files changed, 3733 insertions(+), 847 deletions(-)
> >>
> >> It may have been premature to merge the patches as you say, but now that
> >> they are in, I'd vote for giving Fernando a chance to fix up any damage
> >> that was done in the process rather than just reverting all the useful
> >> changes.
> >
> > In looking at this further, I agree.
> >
> > Felipe, are all of these really needing to be reverted? ?How about
> > picking out the functional changes that need to be resolved instead of
> > just rolling back everything that has been done here. ?Surely not all of
> > these are wrong, right?
>
> Patches are _NOT_ wrong, missing dependencies break the bridge.
> Without that dependencies the first patch of the set won't work and
> all other patches have dependency on the first one, so all of them
> need to be reverted.
How about hand-reverting only the wrong patch, so the other work isn't
lost? I'd much prefer that.
thanks,
greg k-h
* Guzman Lugo, Fernando <[email protected]> [101109 08:36]:
>
> tidspbridge iommu change are working fine all the patches and few fixes after
> that are alredy sent. what breaks tidspbridge, is the unmerged
> dependencies in linux omap tree, specifically the iommu module patches
> and the SG patch.
Care to post a series of the missing patches listed above?
That way we can at least start merging those into linux-omap for
testing while waiting for the next merge window.
Tony
On Tue, Nov 9, 2010 at 11:25 AM, Greg KH <[email protected]> wrote:
> On Tue, Nov 09, 2010 at 11:04:18AM -0600, Guzman Lugo, Fernando wrote:
>> On Tue, Nov 9, 2010 at 10:55 AM, Greg KH <[email protected]> wrote:
>> > On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
>> >> On Tuesday 09 November 2010, Felipe Contreras wrote:
>> >> > Felipe Contreras (14):
>> >> > ? ? ? Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>> >> > ? ? ? Revert "staging: tidspbridge - remove dmm custom module"
>> >> > ? ? ? Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>> >> > ? ? ? Revert "staging: tidspbridge - remove reserved memory clean up"
>> >> > ? ? ? Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>> >> > ? ? ? Revert "staging: tidspbridge - move all iommu related code to a new file"
>> >> > ? ? ? Revert "staging: tidspbridge - remove hw directory"
>> >> > ? ? ? Revert "staging: tidspbridge - fix mmufault support"
>> >> > ? ? ? Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>> >> > ? ? ? Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>> >> > ? ? ? Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>> >> > ? ? ? Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>> >>
>> >> That adds quite a lot of crap back in that was removed by Fernando earlier:
>> >>
>> >> ?44 files changed, 3733 insertions(+), 847 deletions(-)
>> >>
>> >> It may have been premature to merge the patches as you say, but now that
>> >> they are in, I'd vote for giving Fernando a chance to fix up any damage
>> >> that was done in the process rather than just reverting all the useful
>> >> changes.
>> >
>> > In looking at this further, I agree.
>> >
>> > Felipe, are all of these really needing to be reverted? ?How about
>> > picking out the functional changes that need to be resolved instead of
>> > just rolling back everything that has been done here. ?Surely not all of
>> > these are wrong, right?
>>
>> Patches are _NOT_ wrong, missing dependencies break the bridge.
>> Without that dependencies the first patch of the set won't work and
>> all other patches have dependency on the first one, so all of them
>> need to be reverted.
>
> How about hand-reverting only the wrong patch, so the other work isn't
> lost? ?I'd much prefer that.
Unfortunately any of the iommu migration patches will work correctly
without the dependencies on iommu module patches. There are some
patches which cleanup the code, but thanks to the iommu migrations the
files can disappear complete other wise I need to check and only clean
what is not needed and leave what the old custom implementation is
using, which will need a lot of rework in the patches. According with
Felipe Contreras it is very easy reverting and pushing after. I don't
like the idea of reverting all iommu patches, however looks the
easiest solution. Unless the dependencies patches can be merged the
would be the best solution.
Regards,
Fernando.
>
> thanks,
>
> greg k-h
>
On Tue, Nov 9, 2010 at 11:35 AM, Tony Lindgren <[email protected]> wrote:
> * Guzman Lugo, Fernando <[email protected]> [101109 08:36]:
>>
>> tidspbridge iommu change are working fine all the patches and few fixes after
>> that are alredy sent. what breaks tidspbridge, is the unmerged
>> dependencies in linux omap tree, specifically the iommu module patches
>> and the SG patch.
>
> Care to post a series of the missing patches listed above?
Here are the missing patches:
Fernando Guzman Lugo (4):
iovmm: no gap checking for fixed address
iovmm: add superpages support to fixed da address
iovmm: replace __iounmap with omap_iounmap
iommu: create new api to set valid da range
and
scatterlist: define SG chain for arm architecture
Regards,
Fernando.
>
> That way we can at least start merging those into linux-omap for
> testing while waiting for the next merge window.
>
> Tony
>
On Tue, Nov 09, 2010 at 11:49:30AM -0600, Guzman Lugo, Fernando wrote:
> On Tue, Nov 9, 2010 at 11:25 AM, Greg KH <[email protected]> wrote:
> > On Tue, Nov 09, 2010 at 11:04:18AM -0600, Guzman Lugo, Fernando wrote:
> >> On Tue, Nov 9, 2010 at 10:55 AM, Greg KH <[email protected]> wrote:
> >> > On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
> >> >> On Tuesday 09 November 2010, Felipe Contreras wrote:
> >> >> > Felipe Contreras (14):
> >> >> > ? ? ? Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
> >> >> > ? ? ? Revert "staging: tidspbridge - remove dmm custom module"
> >> >> > ? ? ? Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
> >> >> > ? ? ? Revert "staging: tidspbridge - remove reserved memory clean up"
> >> >> > ? ? ? Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
> >> >> > ? ? ? Revert "staging: tidspbridge - move all iommu related code to a new file"
> >> >> > ? ? ? Revert "staging: tidspbridge - remove hw directory"
> >> >> > ? ? ? Revert "staging: tidspbridge - fix mmufault support"
> >> >> > ? ? ? Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
> >> >> > ? ? ? Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
> >> >> > ? ? ? Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
> >> >> > ? ? ? Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
> >> >>
> >> >> That adds quite a lot of crap back in that was removed by Fernando earlier:
> >> >>
> >> >> ?44 files changed, 3733 insertions(+), 847 deletions(-)
> >> >>
> >> >> It may have been premature to merge the patches as you say, but now that
> >> >> they are in, I'd vote for giving Fernando a chance to fix up any damage
> >> >> that was done in the process rather than just reverting all the useful
> >> >> changes.
> >> >
> >> > In looking at this further, I agree.
> >> >
> >> > Felipe, are all of these really needing to be reverted? ?How about
> >> > picking out the functional changes that need to be resolved instead of
> >> > just rolling back everything that has been done here. ?Surely not all of
> >> > these are wrong, right?
> >>
> >> Patches are _NOT_ wrong, missing dependencies break the bridge.
> >> Without that dependencies the first patch of the set won't work and
> >> all other patches have dependency on the first one, so all of them
> >> need to be reverted.
> >
> > How about hand-reverting only the wrong patch, so the other work isn't
> > lost? ?I'd much prefer that.
>
> Unfortunately any of the iommu migration patches will work correctly
> without the dependencies on iommu module patches. There are some
> patches which cleanup the code, but thanks to the iommu migrations the
> files can disappear complete other wise I need to check and only clean
> what is not needed and leave what the old custom implementation is
> using, which will need a lot of rework in the patches. According with
> Felipe Contreras it is very easy reverting and pushing after.
If it is easy to revert and push later, then the "revert just this
piece" should be done now.
Seriously, I'm getting very confused here, and am very annoyed by the
whole thing.
Here's what I don't like:
- the original driver didn't even seem to work properly
- people sent me patches they never tested and broke things even worse
- some people have no respect for the omap maintainers and what they
think about things, or even basic knowledge of the kernel
development cycle.
- I do not have this hardware so I can't test anything.
So, from now on, I'm not taking ANYONES patches for this driver unless
it gets an ack from the driver maintainer, Omar Luna.
Actually, no, I'm not going to take any patch unless it _comes from_
Omar. Omar, please work to queue up patches and test them, and then
send them to me for merging.
Any questions?
If anyone doesn't like this because they feel that the current driver is
broken, well, I can easily solve that by just deleting the whole thing
from the tree right now. Would that be a better idea?
Ugh, what a mess...
greg k-h
* Guzman Lugo, Fernando <[email protected]> [101109 09:43]:
> On Tue, Nov 9, 2010 at 11:35 AM, Tony Lindgren <[email protected]> wrote:
> > * Guzman Lugo, Fernando <[email protected]> [101109 08:36]:
> >>
> >> tidspbridge iommu change are working fine all the patches and few fixes after
> >> that are alredy sent. what breaks tidspbridge, is the unmerged
> >> dependencies in linux omap tree, specifically the iommu module patches
> >> and the SG patch.
> >
> > Care to post a series of the missing patches listed above?
>
> Here are the missing patches:
>
> Fernando Guzman Lugo (4):
> iovmm: no gap checking for fixed address
> iovmm: add superpages support to fixed da address
> iovmm: replace __iounmap with omap_iounmap
> iommu: create new api to set valid da range
Yeah this is stuff for Hiroshi to look at and queue for
2.6.28. No way we can merge these now.
> and
> scatterlist: define SG chain for arm architecture
And this we need to test carefully it's not something we
can just merge.
Has this been tested to work with omap MMC drivers?
I'm not at all convinced the drivers can deal with
chained SG lists.. This may not show up with light
testing, the SG lists can be very small in most cases.
So this would have to be tested to make sure the
the chained SG list handled properly. The same goes
for other omap drivers that may be using SG.
Regards,
Tony
Hi,
> Seriously, I'm getting very confused here, and am very annoyed by the
> whole thing.
>
> Here's what I don't like:
> ?- the original driver didn't even seem to work properly
> ?- people sent me patches they never tested and broke things even worse
> ?- some people have no respect for the omap maintainers and what they
> ? ?think about things, or even basic knowledge of the kernel
> ? ?development cycle.
> ?- I do not have this hardware so I can't test anything.
>
> So, from now on, I'm not taking ANYONES patches for this driver unless
> it gets an ack from the driver maintainer, Omar Luna.
>
> Actually, no, I'm not going to take any patch unless it _comes from_
> Omar. ?Omar, please work to queue up patches and test them, and then
> send them to me for merging.
>
> Any questions?
>
> If anyone doesn't like this because they feel that the current driver is
> broken, well, I can easily solve that by just deleting the whole thing
> from the tree right now. ?Would that be a better idea?
I'm fine to queue the patches.
For now, yes, the driver is/has-been broken, and everybody has been
trying for the last two cycles to fix some dependencies (and
unfortunately we do it at the last minute or even after it can't be
done anymore, *sometimes*)... I have been kindly educated by both LO
and staging tree maintainers that it is just a bad practice to hurry
things up and not comply with the development standards.
If 2.6.36 or .37 tidspbridge is needed I'll keep each branch with
their dependent patches in d.o-z (TI's git server), if anybody
(Felipe) wants to keep their own branches I'm fine with that I can
sync with them privately to hear any concerns or something else.
I'll create the branches and sent an announce to both lists.
All dependencies need to be acked long before 2.6.38-rc1 and it is the
responsibility of the senders to track their acceptance.
Thanks for your suggestions and bearing with this situation, I'll
follow them and solve your dislikes about it (the hardware part it
might not be possible :/).
BTW, the testing environment for tidspbridge will be gst-dsp, any
stress testsuites will be run in addition, I'll detail more of it in
the upcoming mail.
Regards,
Omar
On Tue, Nov 9, 2010 at 7:52 PM, Guzman Lugo, Fernando
<[email protected]> wrote:
> On Tue, Nov 9, 2010 at 11:35 AM, Tony Lindgren <[email protected]> wrote:
>> * Guzman Lugo, Fernando <[email protected]> [101109 08:36]:
>>>
>>> tidspbridge iommu change are working fine all the patches and few fixes after
>>> that are alredy sent. what breaks tidspbridge, is the unmerged
>>> dependencies in linux omap tree, specifically the iommu module patches
>>> and the SG patch.
>>
>> Care to post a series of the missing patches listed above?
>
> Here are the missing patches:
>
> Fernando Guzman Lugo (4):
> iovmm: no gap checking for fixed address
> iovmm: add superpages support to fixed da address
> iovmm: replace __iounmap with omap_iounmap
> iommu: create new api to set valid da range
>
> and
> scatterlist: define SG chain for arm architecture
Those are not the only patches needed, you would also need:
omap: iommu: make iva2 iommu selectable
staging: tidspbridge: enable iva2 iommu
And the one that actually uses the new API for the da range which
hasn't even been submitted to the mailing list.
Personally, I haven't seen the new iommu code working correctly.
--
Felipe Contreras
On Tue, Nov 9, 2010 at 7:58 PM, Greg KH <[email protected]> wrote:
> If it is easy to revert and push later, then the "revert just this
> piece" should be done now.
>
> Seriously, I'm getting very confused here, and am very annoyed by the
> whole thing.
With good reason.
> Here's what I don't like:
> - the original driver didn't even seem to work properly
It _was_ working properly, when it was a separate branch, but the
merge to staging wasn't done correctly, so it has never worked there.
> - people sent me patches they never tested and broke things even worse
Part of the blame goes to TI. Even if you have the hardware, it's not
straightforward to test this. I have struggled to improve the
situation on user-space with the gst-dsp and dsp-tools projects, which
btw have not received support from TI, but apparently they were not
used to test this (neither was the "official" solution from TI which
is much harder to set up).
> - some people have no respect for the omap maintainers and what they
> think about things, or even basic knowledge of the kernel
> development cycle.
I don't think this is the case. The opinion of the OMAP maintainers is
valuable in order to cleanup the mess that is this driver. But _first_
it has to work. Most people are not developers, they just want to use
this driver.
> - I do not have this hardware so I can't test anything.
>
> So, from now on, I'm not taking ANYONES patches for this driver unless
> it gets an ack from the driver maintainer, Omar Luna.
>
> Actually, no, I'm not going to take any patch unless it _comes from_
> Omar. Omar, please work to queue up patches and test them, and then
> send them to me for merging.
>
> Any questions?
You mean after this pull, or should Omar re-send this pull request?
--
Felipe Contreras
On Tue, Nov 9, 2010 at 6:29 PM, Arnd Bergmann <[email protected]> wrote:
> On Tuesday 09 November 2010, Felipe Contreras wrote:
>> Felipe Contreras (14):
>> Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>> Revert "staging: tidspbridge - remove dmm custom module"
>> Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>> Revert "staging: tidspbridge - remove reserved memory clean up"
>> Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>> Revert "staging: tidspbridge - move all iommu related code to a new file"
>> Revert "staging: tidspbridge - remove hw directory"
>> Revert "staging: tidspbridge - fix mmufault support"
>> Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>> Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>> Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>> Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>
> That adds quite a lot of crap back in that was removed by Fernando earlier:
>
> 44 files changed, 3733 insertions(+), 847 deletions(-)
>
> It may have been premature to merge the patches as you say, but now that
> they are in, I'd vote for giving Fernando a chance to fix up any damage
> that was done in the process rather than just reverting all the useful
> changes.
The changes from Fernando require changes in other trees: arm, omap.
These cannot get into 2.6.37, and I have my doubts they would get into
2.6.38, as I haven't seen those ack'ed yet.
Moreover, I have applied all the patches Fernando has sent, and I
still haven't seen this driver working with the new iommu code.
So, sure, these patches are good, we (Nokia), requested them years
ago, but right now there's no way to get this driver working with them
on 2.6.37. I would rather have a working driver for once on a vanilla
kernel. It is really tiring to point people to different places to get
working code depending on which week they ask.
--
Felipe Contreras
On Tue, Nov 09, 2010 at 11:53:29PM +0200, Felipe Contreras wrote:
> On Tue, Nov 9, 2010 at 7:58 PM, Greg KH <[email protected]> wrote:
> > If it is easy to revert and push later, then the "revert just this
> > piece" should be done now.
> >
> > Seriously, I'm getting very confused here, and am very annoyed by the
> > whole thing.
>
> With good reason.
>
> > Here's what I don't like:
> > ?- the original driver didn't even seem to work properly
>
> It _was_ working properly, when it was a separate branch, but the
> merge to staging wasn't done correctly, so it has never worked there.
Well, blame the people who sent it to me then :)
> > ?- people sent me patches they never tested and broke things even worse
>
> Part of the blame goes to TI. Even if you have the hardware, it's not
> straightforward to test this. I have struggled to improve the
> situation on user-space with the gst-dsp and dsp-tools projects, which
> btw have not received support from TI, but apparently they were not
> used to test this (neither was the "official" solution from TI which
> is much harder to set up).
>
> > ?- some people have no respect for the omap maintainers and what they
> > ? ?think about things, or even basic knowledge of the kernel
> > ? ?development cycle.
>
> I don't think this is the case. The opinion of the OMAP maintainers is
> valuable in order to cleanup the mess that is this driver. But _first_
> it has to work. Most people are not developers, they just want to use
> this driver.
>
> > ?- I do not have this hardware so I can't test anything.
> >
> > So, from now on, I'm not taking ANYONES patches for this driver unless
> > it gets an ack from the driver maintainer, Omar Luna.
> >
> > Actually, no, I'm not going to take any patch unless it _comes from_
> > Omar. ?Omar, please work to queue up patches and test them, and then
> > send them to me for merging.
> >
> > Any questions?
>
> You mean after this pull, or should Omar re-send this pull request?
I'm not pulling _anything_ at the moment for this driver.
I want to see patches, not a pull request please, as these need to be
reviewed very closely.
thanks,
greg k-h
On Tue, Nov 9, 2010 at 8:32 PM, Tony Lindgren <[email protected]> wrote:
> * Guzman Lugo, Fernando <[email protected]> [101109 09:43]:
>> On Tue, Nov 9, 2010 at 11:35 AM, Tony Lindgren <[email protected]> wrote:
>> > * Guzman Lugo, Fernando <[email protected]> [101109 08:36]:
>> >>
>> >> tidspbridge iommu change are working fine all the patches and few fixes after
>> >> that are alredy sent. what breaks tidspbridge, is the unmerged
>> >> dependencies in linux omap tree, specifically the iommu module patches
>> >> and the SG patch.
>> >
>> > Care to post a series of the missing patches listed above?
>>
>> Here are the missing patches:
>>
>> Fernando Guzman Lugo (4):
>> iovmm: no gap checking for fixed address
>> iovmm: add superpages support to fixed da address
>> iovmm: replace __iounmap with omap_iounmap
>> iommu: create new api to set valid da range
>
> Yeah this is stuff for Hiroshi to look at and queue for
> 2.6.28. No way we can merge these now.
Right, and he hasn't ack'ed them yet. So there's a chance they don't
get into 2.6.38 either.
>> and
>> scatterlist: define SG chain for arm architecture
>
> And this we need to test carefully it's not something we
> can just merge.
>
> Has this been tested to work with omap MMC drivers?
>
> I'm not at all convinced the drivers can deal with
> chained SG lists.. This may not show up with light
> testing, the SG lists can be very small in most cases.
>
> So this would have to be tested to make sure the
> the chained SG list handled properly. The same goes
> for other omap drivers that may be using SG.
And the change doesn't affect OMAP only, it's for all ARM platforms.
The proposal from Russell was to do it only on OMAP, but I haven't
seen a patch that does that yet. Hopefully this would go into 2.6.38,
but again, it might not.
--
Felipe Contreras
On Wed, Nov 10, 2010 at 12:10 AM, Greg KH <[email protected]> wrote:
> On Tue, Nov 09, 2010 at 11:53:29PM +0200, Felipe Contreras wrote:
>> You mean after this pull, or should Omar re-send this pull request?
>
> I'm not pulling _anything_ at the moment for this driver.
>
> I want to see patches, not a pull request please, as these need to be
> reviewed very closely.
It's a revert from a non-working state to a working state, there's not
much to review there. So if Omar sends the patches with my and his
s-o-b, would you apply the patches?
Getting these patches in one way or the other seems to be the only way
we can get this driver to work on 2.6.37.
--
Felipe Contreras
On Wed, Nov 10, 2010 at 12:39:47AM +0200, Felipe Contreras wrote:
> On Wed, Nov 10, 2010 at 12:10 AM, Greg KH <[email protected]> wrote:
> > On Tue, Nov 09, 2010 at 11:53:29PM +0200, Felipe Contreras wrote:
> >> You mean after this pull, or should Omar re-send this pull request?
> >
> > I'm not pulling _anything_ at the moment for this driver.
> >
> > I want to see patches, not a pull request please, as these need to be
> > reviewed very closely.
>
> It's a revert from a non-working state to a working state, there's not
> much to review there. So if Omar sends the patches with my and his
> s-o-b, would you apply the patches?
Was my previous email about not taking anything unless it went through
Omar not specific enough?