Getting your opinion about the best place to put one specific device driver...

Getting your opinion about the best place to put one specific device driver...

[Jean-Nicolas GRAUX]

Hello, Arnd, Olof. First, let me introduce myself quickly.
I am working in Stericsson and i am a colleague of Linus Walleij.
We are currently doing some cleaning in our mach-ux500 ARM machine.

Among other things, the U8500 SoC and its derivatives embed one small IP 
called the "hardware observer".
This is used for hardware debug purpose and it provides the ability to 
output some
modem, power, clocking, ..., hardware signals on 18 external wires.

We did one small platform device driver to handle this piece of hardware.
In the patch attached to this file, we kept the code in the mach-ux500 
machine folder.
But we are wondering where is the best place to put that stuff.
So, the question is: where should we put this code in the kernel tree ?

Thanks in advance for your advises.
Regards. Jean-Nicolas.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ARM-ux500-introduce-hardware-observer-platform-drive.patch
Type: text/x-diff
Size: 27999 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130212/4c348ce9/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-ARM-ux500-remove-hardware-observer-machine-code.patch
Type: text/x-diff
Size: 25978 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130212/4c348ce9/attachment-0003.bin>

Getting your opinion about the best place to put one specific device driver...

[Arnd Bergmann]

On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
> ello, Arnd, Olof. First, let me introduce myself quickly.
> I am working in Stericsson and i am a colleague of Linus Walleij.
> We are currently doing some cleaning in our mach-ux500 ARM machine.
> 
> Among other things, the U8500 SoC and its derivatives embed one small IP 
> called the "hardware observer".
> This is used for hardware debug purpose and it provides the ability to 
> output some
> modem, power, clocking, ..., hardware signals on 18 external wires.
> 
> We did one small platform device driver to handle this piece of hardware.
> In the patch attached to this file, we kept the code in the mach-ux500 
> machine folder.
> But we are wondering where is the best place to put that stuff.
> So, the question is: where should we put this code in the kernel tree ?
 
Hi Jean-Nicolas,

I think I need some more information to understand what that interface
you are driving is actually about, since that is not clear from your
description or from reading the source code.

Why are there exactly 18 wires?

What is the protocol that is used on these wires (i2c, spi, rs232, ...)

Why do you actually need run-time configuration in the kernel? 

It does look however like this code is related to the PRCMU, so maybe
it should be part of the prcmu driver rather than a separate device?

	Arnd

Getting your opinion about the best place to put one specific device driver...

[Jean-Nicolas GRAUX]

Arnd, here are my answers below.

Le 02/12/2013 03:54 PM, Arnd Bergmann a ?crit :
> On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
>> ello, Arnd, Olof. First, let me introduce myself quickly.
>> I am working in Stericsson and i am a colleague of Linus Walleij.
>> We are currently doing some cleaning in our mach-ux500 ARM machine.
>>
>> Among other things, the U8500 SoC and its derivatives embed one small IP
>> called the "hardware observer".
>> This is used for hardware debug purpose and it provides the ability to
>> output some
>> modem, power, clocking, ..., hardware signals on 18 external wires.
>>
>> We did one small platform device driver to handle this piece of hardware.
>> In the patch attached to this file, we kept the code in the mach-ux500
>> machine folder.
>> But we are wondering where is the best place to put that stuff.
>> So, the question is: where should we put this code in the kernel tree ?
>   
> Hi Jean-Nicolas,
>
> I think I need some more information to understand what that interface
> you are driving is actually about, since that is not clear from your
> description or from reading the source code.
>
> Why are there exactly 18 wires?
As you can imagine, this module is very specific to the ux500 digital 
baseband family.
So, ux500 SoCs simply provide 18 external IOs that may be used to 
observe some critical hardware signals.
Depending on what has been configured in the hwobs control registers, we 
are able to observe
signals from the modem, the ddr controllers, the prcmu, the gfx ip, the 
clock tree, ...
>
> What is the protocol that is used on these wires (i2c, spi, rs232, ...)
There is no specific devices connected behind those wires. So, no 
protocol used.
Usually, once the required configuration has been set thanks to the 
debugfs interface,
we directly monitor the signals by connecting the wires to a 
oscilloscope or a digital analyser...
>
> Why do you actually need run-time configuration in the kernel?
Main aim of this driver is just to provide a user interface to 
configure/enable the hardware observer
so that we may easily select the required signals to observe. Having 
that at run time is mandatory for us.
This is mainly used by software/hardware teams to debug and verify 
modem/ape power states.
>
> It does look however like this code is related to the PRCMU, so maybe
> it should be part of the prcmu driver rather than a separate device?
It is true that the hardware observer registers are located in the 
"PRCM" unit.
But to my mind, it has no real link with the dbx500-prcmu driver itself yet.
(dbx500-prcmu driver is dedicated to the handling of the communication
between the kernel and the firmware that run inside PRCMU xp70 controller.)
Moreover, I think we should keep a separate device for the harware observer
since it need to acquire its own pinctrl state.
That said, we might consider moving it to the mfd folder ?

To be honest, in my initial patch, i was aiming to put the ux500_hwobs.* 
files in the "misc" folder.
But Linus told me that this was probably not the good place ;)
Regards.
>
> 	Arnd

Getting your opinion about the best place to put one specific device driver...

[Tony Lindgren]

Hi,

* Jean-Nicolas GRAUX <jean-nicolas.graux@stericsson.com> [130212 08:33]:
> Arnd, here are my answers below.
> 
> Le 02/12/2013 03:54 PM, Arnd Bergmann a ?crit :
> >On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
> >>ello, Arnd, Olof. First, let me introduce myself quickly.
> >>I am working in Stericsson and i am a colleague of Linus Walleij.
> >>We are currently doing some cleaning in our mach-ux500 ARM machine.
> >>
> >>Among other things, the U8500 SoC and its derivatives embed one small IP
> >>called the "hardware observer".
> >>This is used for hardware debug purpose and it provides the ability to
> >>output some
> >>modem, power, clocking, ..., hardware signals on 18 external wires.
> >>
> >>We did one small platform device driver to handle this piece of hardware.
> >>In the patch attached to this file, we kept the code in the mach-ux500
> >>machine folder.
> >>But we are wondering where is the best place to put that stuff.
> >>So, the question is: where should we put this code in the kernel tree ?
> >Hi Jean-Nicolas,
> >
> >I think I need some more information to understand what that interface
> >you are driving is actually about, since that is not clear from your
> >description or from reading the source code.
> >
> >Why are there exactly 18 wires?
> As you can imagine, this module is very specific to the ux500
> digital baseband family.
> So, ux500 SoCs simply provide 18 external IOs that may be used to
> observe some critical hardware signals.
> Depending on what has been configured in the hwobs control
> registers, we are able to observe
> signals from the modem, the ddr controllers, the prcmu, the gfx ip,
> the clock tree, ...

Sounds very similar to what we have on omaps as hwobs. AFAIK that
module is just multiplexing signals. So maybe take a look how it
would fit into drivers/pinctrl?

At least the omap hwobs could in theory already be handled by
pinctrl-single.c.

Regards,

Tony

Getting your opinion about the best place to put one specific device driver...

[Arnd Bergmann]

On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
> Le 02/12/2013 03:54 PM, Arnd Bergmann a ?crit :
> > On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
> > I think I need some more information to understand what that interface
> > you are driving is actually about, since that is not clear from your
> > description or from reading the source code.
> >
> > Why are there exactly 18 wires?
> As you can imagine, this module is very specific to the ux500 digital 
> baseband family.
> So, ux500 SoCs simply provide 18 external IOs that may be used to 
> observe some critical hardware signals.
> Depending on what has been configured in the hwobs control registers, we 
> are able to observe
> signals from the modem, the ddr controllers, the prcmu, the gfx ip, the 
> clock tree, ...

Ah, so these are signals that are internal to the SoC and that normally
don't need to get routed to a pad unless you want to monitor them, right?

> > What is the protocol that is used on these wires (i2c, spi, rs232, ...)
> There is no specific devices connected behind those wires. So, no 
> protocol used.
> Usually, once the required configuration has been set thanks to the 
> debugfs interface,
> we directly monitor the signals by connecting the wires to a 
> oscilloscope or a digital analyser...

Ok.

> > Why do you actually need run-time configuration in the kernel?
> Main aim of this driver is just to provide a user interface to 
> configure/enable the hardware observer
> so that we may easily select the required signals to observe. Having 
> that at run time is mandatory for us.
> This is mainly used by software/hardware teams to debug and verify 
> modem/ape power states.

Ok, I see.

> > It does look however like this code is related to the PRCMU, so maybe
> > it should be part of the prcmu driver rather than a separate device?
>
> It is true that the hardware observer registers are located in the 
> "PRCM" unit.
> But to my mind, it has no real link with the dbx500-prcmu driver itself yet.
> (dbx500-prcmu driver is dedicated to the handling of the communication
> between the kernel and the firmware that run inside PRCMU xp70 controller.)
> Moreover, I think we should keep a separate device for the harware observer
> since it need to acquire its own pinctrl state.
> That said, we might consider moving it to the mfd folder ?
> 
> To be honest, in my initial patch, i was aiming to put the ux500_hwobs.* 
> files in the "misc" folder.
> But Linus told me that this was probably not the good place ;)

Correct, I try my best to avoid adding random stuff in there that might
need a generic interface later. Tony already pointed out the similarity
to the OMAP specific hwopbs feature, and whatever user interface we
introduce for one should be generic enough to work on the other one as
well, and ideally also on future ones to the degree that we can anticipate
them today.

I like the idea of making the in-kernel configuration part of this a
pinctrl driver, which would already allow you to configure it through
a custom device tree and no user interface at all.

If a more dynamic user interface is needed, I think it would be good
to have a generic mechanism in the pinctrl subsystem to reroute pins
like these, which can work for all pins in the system (or at least
those that have not been claimed by another device). I don't know if
that interface already exists, but Linus would be the right person
to answer that.

If there is a generic way to configure pinmux from user space, you
can essentially move this driver into an application running outside
of the kernel.

	Arnd

Getting your opinion about the best place to put one specific device driver...

[Jean-Nicolas GRAUX]

Le 02/12/2013 06:41 PM, Arnd Bergmann a ?crit :
> On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
>> Le 02/12/2013 03:54 PM, Arnd Bergmann a ?crit :
>>> On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
>>> I think I need some more information to understand what that interface
>>> you are driving is actually about, since that is not clear from your
>>> description or from reading the source code.
>>>
>>> Why are there exactly 18 wires?
>> As you can imagine, this module is very specific to the ux500 digital
>> baseband family.
>> So, ux500 SoCs simply provide 18 external IOs that may be used to
>> observe some critical hardware signals.
>> Depending on what has been configured in the hwobs control registers, we
>> are able to observe
>> signals from the modem, the ddr controllers, the prcmu, the gfx ip, the
>> clock tree, ...
> Ah, so these are signals that are internal to the SoC and that normally
> don't need to get routed to a pad unless you want to monitor them, right?
Yes.
>>> What is the protocol that is used on these wires (i2c, spi, rs232, ...)
>> There is no specific devices connected behind those wires. So, no
>> protocol used.
>> Usually, once the required configuration has been set thanks to the
>> debugfs interface,
>> we directly monitor the signals by connecting the wires to a
>> oscilloscope or a digital analyser...
> Ok.
>
>>> Why do you actually need run-time configuration in the kernel?
>> Main aim of this driver is just to provide a user interface to
>> configure/enable the hardware observer
>> so that we may easily select the required signals to observe. Having
>> that at run time is mandatory for us.
>> This is mainly used by software/hardware teams to debug and verify
>> modem/ape power states.
> Ok, I see.
>
>>> It does look however like this code is related to the PRCMU, so maybe
>>> it should be part of the prcmu driver rather than a separate device?
>> It is true that the hardware observer registers are located in the
>> "PRCM" unit.
>> But to my mind, it has no real link with the dbx500-prcmu driver itself yet.
>> (dbx500-prcmu driver is dedicated to the handling of the communication
>> between the kernel and the firmware that run inside PRCMU xp70 controller.)
>> Moreover, I think we should keep a separate device for the harware observer
>> since it need to acquire its own pinctrl state.
>> That said, we might consider moving it to the mfd folder ?
>>
>> To be honest, in my initial patch, i was aiming to put the ux500_hwobs.*
>> files in the "misc" folder.
>> But Linus told me that this was probably not the good place ;)
> Correct, I try my best to avoid adding random stuff in there that might
> need a generic interface later. Tony already pointed out the similarity
> to the OMAP specific hwopbs feature, and whatever user interface we
> introduce for one should be generic enough to work on the other one as
> well, and ideally also on future ones to the degree that we can anticipate
> them today.
>
> I like the idea of making the in-kernel configuration part of this a
> pinctrl driver, which would already allow you to configure it through
> a custom device tree and no user interface at all.
Yes, that would be nice for sure.
> If a more dynamic user interface is needed, I think it would be good
> to have a generic mechanism in the pinctrl subsystem to reroute pins
> like these, which can work for all pins in the system (or at least
> those that have not been claimed by another device). I don't know if
> that interface already exists, but Linus would be the right person
> to answer that.
I think i have one colleague that recently made a patch for that purpose.
But i believe this is still in progress. It consists in improving the 
pinctrl
debugfs interface to be able to manually change the pin muxing and
the gpio configuration for debug/testing purpose.
> If there is a generic way to configure pinmux from user space, you
> can essentially move this driver into an application running outside
> of the kernel.
>
> 	Arnd
The matter is that this hwobs driver do more than just changing
the muxing of one pin group in the digital baseband.
It also control some specific registers to configure the hwobs IP.
More in details, for each of the 18 external IOs, it is possible to
select one observer mode among several. We also provide some
facility for the user to easily detect the pin to monitor on the
output connector.

Anyway, maybe this could be handled in one dedicated pinctrl
driver as suggested by Tony.

Thank you for your advises.
Jean-Nicolas.

Getting your opinion about the best place to put one specific device driver...

[Arnd Bergmann]

On Wednesday 13 February 2013, Jean-Nicolas GRAUX wrote:
> Le 02/12/2013 06:41 PM, Arnd Bergmann a ?crit :
> > On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
> > If a more dynamic user interface is needed, I think it would be good
> > to have a generic mechanism in the pinctrl subsystem to reroute pins
> > like these, which can work for all pins in the system (or at least
> > those that have not been claimed by another device). I don't know if
> > that interface already exists, but Linus would be the right person
> > to answer that.
>
> I think i have one colleague that recently made a patch for that purpose.
> But i believe this is still in progress. It consists in improving the 
> pinctrl
> debugfs interface to be able to manually change the pin muxing and
> the gpio configuration for debug/testing purpose.

Sounds good to me. Peter, Tony, do you think that would be a good enough
interface for other platforms as well?

> > If there is a generic way to configure pinmux from user space, you
> > can essentially move this driver into an application running outside
> > of the kernel.
> >
> The matter is that this hwobs driver do more than just changing
> the muxing of one pin group in the digital baseband.
> It also control some specific registers to configure the hwobs IP.
> More in details, for each of the 18 external IOs, it is possible to
> select one observer mode among several. We also provide some
> facility for the user to easily detect the pin to monitor on the
> output connector.

Hmm, I wonder if that is something that makes sense to have
in a generic pinctrl debugfs interface. Maybe it can instead
be part of the new pinctrl driver for this hw block then, which
should be able to create additional sysfs files in the pinctrl
debugfs directories. Linus, does that make sense to you?

	Arnd

Getting your opinion about the best place to put one specific device driver...

[Tony Lindgren]

* Arnd Bergmann <arnd@arndb.de> [130213 03:07]:
> On Wednesday 13 February 2013, Jean-Nicolas GRAUX wrote:
> > Le 02/12/2013 06:41 PM, Arnd Bergmann a ?crit :
> > > On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
> > > If a more dynamic user interface is needed, I think it would be good
> > > to have a generic mechanism in the pinctrl subsystem to reroute pins
> > > like these, which can work for all pins in the system (or at least
> > > those that have not been claimed by another device). I don't know if
> > > that interface already exists, but Linus would be the right person
> > > to answer that.
> >
> > I think i have one colleague that recently made a patch for that purpose.
> > But i believe this is still in progress. It consists in improving the 
> > pinctrl
> > debugfs interface to be able to manually change the pin muxing and
> > the gpio configuration for debug/testing purpose.
> 
> Sounds good to me. Peter, Tony, do you think that would be a good enough
> interface for other platforms as well?

Yes debugfs is the way to go for modifying the hwobs settings.
 
> > > If there is a generic way to configure pinmux from user space, you
> > > can essentially move this driver into an application running outside
> > > of the kernel.
> > >
> > The matter is that this hwobs driver do more than just changing
> > the muxing of one pin group in the digital baseband.
> > It also control some specific registers to configure the hwobs IP.
> > More in details, for each of the 18 external IOs, it is possible to
> > select one observer mode among several. We also provide some
> > facility for the user to easily detect the pin to monitor on the
> > output connector.
> 
> Hmm, I wonder if that is something that makes sense to have
> in a generic pinctrl debugfs interface. Maybe it can instead
> be part of the new pinctrl driver for this hw block then, which
> should be able to create additional sysfs files in the pinctrl
> debugfs directories. Linus, does that make sense to you?

Makes sense to me.

And pinctrl does not need to have any knowledge of the names of
the hwobs signals etc. That can all be done in a user space app
modifying the pinctrl registers via debugs.

Regards,

Tony

Getting your opinion about the best place to put one specific device driver...

[Peter De Schrijver]

On Wed, Feb 13, 2013 at 05:54:39PM +0100, Tony Lindgren wrote:
> * Arnd Bergmann <arnd@arndb.de> [130213 03:07]:
> > On Wednesday 13 February 2013, Jean-Nicolas GRAUX wrote:
> > > Le 02/12/2013 06:41 PM, Arnd Bergmann a ?crit :
> > > > On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
> > > > If a more dynamic user interface is needed, I think it would be good
> > > > to have a generic mechanism in the pinctrl subsystem to reroute pins
> > > > like these, which can work for all pins in the system (or at least
> > > > those that have not been claimed by another device). I don't know if
> > > > that interface already exists, but Linus would be the right person
> > > > to answer that.
> > >
> > > I think i have one colleague that recently made a patch for that purpose.
> > > But i believe this is still in progress. It consists in improving the 
> > > pinctrl
> > > debugfs interface to be able to manually change the pin muxing and
> > > the gpio configuration for debug/testing purpose.
> > 
> > Sounds good to me. Peter, Tony, do you think that would be a good enough
> > interface for other platforms as well?
> 
> Yes debugfs is the way to go for modifying the hwobs settings.

Agreed. debugfs would be fine.

Cheers,

Peter.

Getting your opinion about the best place to put one specific device driver...

[Linus Walleij]

On Wed, Feb 13, 2013 at 12:04 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 13 February 2013, Jean-Nicolas GRAUX wrote:
>> Le 02/12/2013 06:41 PM, Arnd Bergmann a ?crit :
>> > On Tuesday 12 February 2013, Jean-Nicolas GRAUX wrote:
>> > If a more dynamic user interface is needed, I think it would be good
>> > to have a generic mechanism in the pinctrl subsystem to reroute pins
>> > like these, which can work for all pins in the system (or at least
>> > those that have not been claimed by another device). I don't know if
>> > that interface already exists, but Linus would be the right person
>> > to answer that.
>>
>> I think i have one colleague that recently made a patch for that purpose.
>> But i believe this is still in progress. It consists in improving the
>> pinctrl
>> debugfs interface to be able to manually change the pin muxing and
>> the gpio configuration for debug/testing purpose.
>
> Sounds good to me. Peter, Tony, do you think that would be a good enough
> interface for other platforms as well?

We're talking about this:
http://marc.info/?l=linux-kernel&m=136052713530502&w=2

We're discussing it right now but Stephen Warren has some
valid concerns so I'm pondering the right way forward.

However that patch is about pin configuration, which is
dealing with an opaque unsigned long. This usecase would
be a debugfs interface to set up muxing and a totally different
piece of code in drivers/pinctrl/pinmux.c. But I think it's
a good idea!

Traditionally, hardware observation and stuff like this has been
kept out of the mainline kernel, and the fact that we're getting to
it now is due to the fact that all the basic stuff is already quite
nicely lined up. So it's basically a good sign.

>> > If there is a generic way to configure pinmux from user space, you
>> > can essentially move this driver into an application running outside
>> > of the kernel.
>> >
>> The matter is that this hwobs driver do more than just changing
>> the muxing of one pin group in the digital baseband.
>> It also control some specific registers to configure the hwobs IP.
>> More in details, for each of the 18 external IOs, it is possible to
>> select one observer mode among several. We also provide some
>> facility for the user to easily detect the pin to monitor on the
>> output connector.
>
> Hmm, I wonder if that is something that makes sense to have
> in a generic pinctrl debugfs interface. Maybe it can instead
> be part of the new pinctrl driver for this hw block then, which
> should be able to create additional sysfs files in the pinctrl
> debugfs directories. Linus, does that make sense to you?

That is one way to achieve it, like this driver spawns some
custom debugfs entries, fine.

The hardware observer already use the pinctrl interfaces from
pinctrl-nomadik.c to set up the pins per se: i.e. to set up so
that these pins are available to put signals on. But then we
can plug this stuff as a front-end, and yeah.
drivers/pinctrl/ux500-hwobs.c might be a good idea!

Yours,
Linus Walleij

Getting your opinion about the best place to put one specific device driver...

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [130214 00:56]:
> 
> The hardware observer already use the pinctrl interfaces from
> pinctrl-nomadik.c to set up the pins per se: i.e. to set up so
> that these pins are available to put signals on. But then we
> can plug this stuff as a front-end, and yeah.
> drivers/pinctrl/ux500-hwobs.c might be a good idea!

Just please try to keep the data added to kernel to minimum..

The user space app can decipher what each register means
based on the register address + SoC model. Kernel just needs
to be able to route the signals.

Otherwise we'll end up with data for each SoC in the kernel,
which we're trying to avoid :)

Regards,

Tony

Getting your opinion about the best place to put one specific device driver...

[Peter De Schrijver]

> 
> Correct, I try my best to avoid adding random stuff in there that might
> need a generic interface later. Tony already pointed out the similarity
> to the OMAP specific hwopbs feature, and whatever user interface we
> introduce for one should be generic enough to work on the other one as
> well, and ideally also on future ones to the degree that we can anticipate
> them today.
> 
> I like the idea of making the in-kernel configuration part of this a
> pinctrl driver, which would already allow you to configure it through
> a custom device tree and no user interface at all.
> 

Having done significant work with the OMAP3 hwobs feature, you really want a
userinterface. At least OMAP3 has limitations on which signal can be
monitored on which hwobs pin. This means that depending on the measurements
you want to do, you need to change the configuration. If you want to automate
several measurements, having a user interface (eg using debugfs), makes things
much easier. Also when using this for debugging, you probably will have to
experiment with which signals to monitor to rootcause the problem. Also then
having a userinterface will be much easier than modifying the devicetree
and rebooting the system.

Cheers,

Peter.

Getting your opinion about the best place to put one specific device driver...

[Jean-Nicolas GRAUX]

Le 02/13/2013 10:45 AM, Peter De Schrijver a ?crit :
>> Correct, I try my best to avoid adding random stuff in there that might
>> need a generic interface later. Tony already pointed out the similarity
>> to the OMAP specific hwopbs feature, and whatever user interface we
>> introduce for one should be generic enough to work on the other one as
>> well, and ideally also on future ones to the degree that we can anticipate
>> them today.
>>
>> I like the idea of making the in-kernel configuration part of this a
>> pinctrl driver, which would already allow you to configure it through
>> a custom device tree and no user interface at all.
>>
> Having done significant work with the OMAP3 hwobs feature, you really want a
> userinterface. At least OMAP3 has limitations on which signal can be
> monitored on which hwobs pin. This means that depending on the measurements
> you want to do, you need to change the configuration. If you want to automate
> several measurements, having a user interface (eg using debugfs), makes things
> much easier. Also when using this for debugging, you probably will have to
> experiment with which signals to monitor to rootcause the problem. Also then
> having a userinterface will be much easier than modifying the devicetree
> and rebooting the system.
>
> Cheers,
>
> Peter.
>
Yep, that's true.

Getting your opinion about the best place to put one specific device driver...

[Tony Lindgren]

* Jean-Nicolas GRAUX <jean-nicolas.graux@stericsson.com> [130213 01:52]:
> Le 02/13/2013 10:45 AM, Peter De Schrijver a ?crit :
> >>Correct, I try my best to avoid adding random stuff in there that might
> >>need a generic interface later. Tony already pointed out the similarity
> >>to the OMAP specific hwopbs feature, and whatever user interface we
> >>introduce for one should be generic enough to work on the other one as
> >>well, and ideally also on future ones to the degree that we can anticipate
> >>them today.
> >>
> >>I like the idea of making the in-kernel configuration part of this a
> >>pinctrl driver, which would already allow you to configure it through
> >>a custom device tree and no user interface at all.
> >>
> >Having done significant work with the OMAP3 hwobs feature, you really want a
> >userinterface. At least OMAP3 has limitations on which signal can be
> >monitored on which hwobs pin. This means that depending on the measurements
> >you want to do, you need to change the configuration. If you want to automate
> >several measurements, having a user interface (eg using debugfs), makes things
> >much easier. Also when using this for debugging, you probably will have to
> >experiment with which signals to monitor to rootcause the problem. Also then
> >having a userinterface will be much easier than modifying the devicetree
> >and rebooting the system.
> >
> Yep, that's true.

Yeah user space is the way to go for toggling the settings. For omap hwobs,
that all should be pretty close doable via debugfs with pinctrl-single.c +
the patches posted by Haojian to add debugfs write support.

Regards,

Tony

Getting your opinion about the best place to put one specific device driver...

[Russell King - ARM Linux]

On Tue, Feb 12, 2013 at 10:44:49AM +0100, Jean-Nicolas GRAUX wrote:
> We did one small platform device driver to handle this piece of hardware.
> In the patch attached to this file, we kept the code in the mach-ux500  
> machine folder.
> But we are wondering where is the best place to put that stuff.
> So, the question is: where should we put this code in the kernel tree ?

That is one of the most difficult questions...

Definitely not in arch/arm.  Device drivers do not belong under arch/ but
under the drivers/ subtree.  Several possibilities:

drivers/platform/<appropriate-new-subdirectory>
drivers/misc/<appropriate-new-subdirectory>
drivers/misc

Also, consider getting rid of:
> ST-Ericsson-ID: 478266
> ST-Ericsson-FOSS-OUT-ID: Trivial

from your commit messages when you submit this upstream.

> +static const char readme[] =
> +	"\nHardware observer usage:\n\n"
> +	"./available_modes: List supported hardware observer modes.\n"
> +	"./ddr_select:      Set/get current ddr instance (0|1) to monitor.\n"
> +	"./enable:          Enable/disable hardware observer.\n"
> +	"./gpio_x:          Configure hardware observer IO number x in gpio output:\n"
> +	"                   Write IO number followed by 1 or 0 to select high/low output.\n"
> +	"                   example: echo \"12 1\" > gpio_x\n"
> +	"./mode:            Force a mode to all hardware observer IOs.\n"
> +	"./mode_x:          Set mode to one harware observer IO:\n"
> +	"                   Write IO number followed by mode to set current io mode:\n"
> +	"                   example: echo \"12 wake-up\" > mode_x\n"
> +	"./status:          Show current mode of all hardware observer IOs.\n";
> +
> +static int hwobs_readme(struct seq_file *s, void *p)
> +{
> +	return seq_printf(s, "%s", readme);
> +}

Eww, yuck, no.  We do not do stuff like that.  Put this in a file under
Documentation/ documenting the interface rather than littering the
debugfs with such things.  I'm sure if you started something like a
Documentation/filesystems/debugfs/ directory, you may make some new
friends!