LinuxLists.cc - Re: [PATCH v2] ADP1653 board code for Nokia RX-51

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

On Wednesday 06 March 2013 21:12:06 Pali Rohár wrote:
> On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> > Hi,
> >
> > On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Rohár wrote:
> > > +/*
> > > + * arch/arm/mach-omap2/board-rx51-camera.c
> > > + *
> > > + * Copyright (C) 2008 Nokia Corporation
> > > + *
> > > + * Contact: Sakari Ailus <[email protected]>
> > > + * Tuukka Toivonen <[email protected]>
> >
> > You should put these people to CC... Just to see if the
> > addresses are still valid (which I doubt).
>
> Ok, trying :-)
>

I got "Delivery Status Notification (Failure)" for both addresses.

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2013-03-06 21:44:53

by Sebastian Reichel

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Roh?r wrote:
> On Wednesday 06 March 2013 21:12:06 Pali Roh?r wrote:
> > On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> > > On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Roh?r wrote:
> > > > +/*
> > > > + * arch/arm/mach-omap2/board-rx51-camera.c
> > > > + *
> > > > + * Copyright (C) 2008 Nokia Corporation
> > > > + *
> > > > + * Contact: Sakari Ailus <[email protected]>
> > > > + * Tuukka Toivonen <[email protected]>
> > >
> > > You should put these people to CC... Just to see if the
> > > addresses are still valid (which I doubt).
> >
> > Ok, trying :-)
> >
>
> I got "Delivery Status Notification (Failure)" for both addresses.

Sakari Ailus hosts some code on github [0], which has the following
email address: [email protected]

I added it to this mail's CC.

[0] https://gitorious.org/~sailus

-- Sebastian

Attachments:

(No filename) (927.00 B)
signature.asc (836.00 B)
Digital signature Download all attachments

2013-03-07 22:18:35

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

Hi guys,

On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian Reichel wrote:
> On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Roh?r wrote:
> > On Wednesday 06 March 2013 21:12:06 Pali Roh?r wrote:
> > > On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> > > > On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Roh?r wrote:
> > > > > +/*
> > > > > + * arch/arm/mach-omap2/board-rx51-camera.c
> > > > > + *
> > > > > + * Copyright (C) 2008 Nokia Corporation
> > > > > + *
> > > > > + * Contact: Sakari Ailus <[email protected]>
> > > > > + * Tuukka Toivonen <[email protected]>
> > > >
> > > > You should put these people to CC... Just to see if the
> > > > addresses are still valid (which I doubt).
> > >
> > > Ok, trying :-)
> > >
> >
> > I got "Delivery Status Notification (Failure)" for both addresses.

This is expected.

> Sakari Ailus hosts some code on github [0], which has the following
> email address: [email protected]
>
> I added it to this mail's CC.
>
> [0] https://gitorious.org/~sailus

Nice to hear people are interested in this. ;-)

The primary reason I haven't tried submitting this to mainline is that ARM
board code has a bad reputation these days. The N900 does not have yet
support for device tree (AFAIK), which also would require a few bits and
pieces on the flash driver to work.

Also the sensor and lens drivers would need at least some work before being
ready for submission to mainline for camera to be usable. Unfortunately I
haven't had recently time to work on this. N9(50) support has higher
priority for myself. That, too, is pending the DT support for the device.

There's indeed more up-to-date code in my repository. Even if it's not too
close to mainline anymore it should be a better starting point than the old
kernel from MeeGo.

<URL:https://gitorious.org/omap3camera/pages/Home>

--
Kind regards,

Sakari Ailus
e-mail: [email protected] XMPP: [email protected]

2013-03-24 14:19:55

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

On Thursday 07 March 2013 23:18:27 Sakari Ailus wrote:
> Hi guys,
>
> On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian Reichel
wrote:
> > On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Rohár wrote:
> > > On Wednesday 06 March 2013 21:12:06 Pali Rohár wrote:
> > > > On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> > > > > On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Rohár
wrote:
> > > > > > +/*
> > > > > > + * arch/arm/mach-omap2/board-rx51-camera.c
> > > > > > + *
> > > > > > + * Copyright (C) 2008 Nokia Corporation
> > > > > > + *
> > > > > > + * Contact: Sakari Ailus <[email protected]>
> > > > > > + * Tuukka Toivonen
> > > > > > <[email protected]>
> > > > >
> > > > > You should put these people to CC... Just to see if
> > > > > the addresses are still valid (which I doubt).
> > > >
> > > > Ok, trying :-)
> > >
> > > I got "Delivery Status Notification (Failure)" for both
> > > addresses.
>
> This is expected.
>
> > Sakari Ailus hosts some code on github [0], which has the
> > following email address:
> > [email protected]
> >
> > I added it to this mail's CC.
> >
> > [0] https://gitorious.org/~sailus
>
> Nice to hear people are interested in this. ;-)
>
> The primary reason I haven't tried submitting this to mainline
> is that ARM board code has a bad reputation these days. The
> N900 does not have yet support for device tree (AFAIK), which
> also would require a few bits and pieces on the flash driver
> to work.
>
> Also the sensor and lens drivers would need at least some work
> before being ready for submission to mainline for camera to
> be usable. Unfortunately I haven't had recently time to work
> on this. N9(50) support has higher priority for myself. That,
> too, is pending the DT support for the device.
>
> There's indeed more up-to-date code in my repository. Even if
> it's not too close to mainline anymore it should be a better
> starting point than the old kernel from MeeGo.
>
> <URL:https://gitorious.org/omap3camera/pages/Home>

Hi,

this board code is same in your git repository and on meego obs.

Patch only adding support for adp1653 driver which is already in
upstream kernel.

Are there any other problems with this patch to go for upstream?

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2013-03-24 21:41:55

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

Hi Pali,

Pali Roh?r wrote:
> On Thursday 07 March 2013 23:18:27 Sakari Ailus wrote:
>> Hi guys,
>>
>> On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian Reichel
> wrote:
>>> On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Roh?r wrote:
>>>> On Wednesday 06 March 2013 21:12:06 Pali Roh?r wrote:
>>>>> On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
>>>>>> On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Roh?r
> wrote:
>>>>>>> +/*
>>>>>>> + * arch/arm/mach-omap2/board-rx51-camera.c
>>>>>>> + *
>>>>>>> + * Copyright (C) 2008 Nokia Corporation
>>>>>>> + *
>>>>>>> + * Contact: Sakari Ailus <[email protected]>
>>>>>>> + * Tuukka Toivonen
>>>>>>> <[email protected]>
>>>>>>
>>>>>> You should put these people to CC... Just to see if
>>>>>> the addresses are still valid (which I doubt).
>>>>>
>>>>> Ok, trying :-)
>>>>
>>>> I got "Delivery Status Notification (Failure)" for both
>>>> addresses.
>>
>> This is expected.
>>
>>> Sakari Ailus hosts some code on github [0], which has the
>>> following email address:
>>> [email protected]
>>>
>>> I added it to this mail's CC.
>>>
>>> [0] https://gitorious.org/~sailus
>>
>> Nice to hear people are interested in this. ;-)
>>
>> The primary reason I haven't tried submitting this to mainline
>> is that ARM board code has a bad reputation these days. The
>> N900 does not have yet support for device tree (AFAIK), which
>> also would require a few bits and pieces on the flash driver
>> to work.
>>
>> Also the sensor and lens drivers would need at least some work
>> before being ready for submission to mainline for camera to
>> be usable. Unfortunately I haven't had recently time to work
>> on this. N9(50) support has higher priority for myself. That,
>> too, is pending the DT support for the device.
>>
>> There's indeed more up-to-date code in my repository. Even if
>> it's not too close to mainline anymore it should be a better
>> starting point than the old kernel from MeeGo.
>>
>> <URL:https://gitorious.org/omap3camera/pages/Home>
>
> Hi,
>
> this board code is same in your git repository and on meego obs.
>
> Patch only adding support for adp1653 driver which is already in
> upstream kernel.
>
> Are there any other problems with this patch to go for upstream?

A few more things comes to mind. We depend a little bit on actual board
code; it's not only static data. That's the configuration of the
external clock from the ISP to the sensor. That should move to the
common clock framework so that the ISP registers the clock and the
sensor driver can then use it. AFAIR Laurent has done some work on that.

The peculiar detail of the rx51 is that there's a switch on the camera
CCP2 bus that selects either sensor (primary or secondary). Both sensors
are connected to the same receiver. That isn't properly modelled
currently at all, so that's why we have rx51_camera_set_xshutdown(). I
guess it'd still work if you only power (i.e. open) either of the
devices at a time, though.

That should be all.

--
Kind regards,

Sakari Ailus
[email protected]

2013-03-25 23:06:15

by Laurent Pinchart

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

Hi Sakari,

On Sunday 24 March 2013 23:46:01 Sakari Ailus wrote:
> Pali Roh?r wrote:
> > On Thursday 07 March 2013 23:18:27 Sakari Ailus wrote:
> >> On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian Reichel wrote:
> >>> On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Roh?r wrote:
> >>>> On Wednesday 06 March 2013 21:12:06 Pali Roh?r wrote:
> >>>>> On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> >>>>>> On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Roh?r wrote:
> >>>>>>> +/*
> >>>>>>> + * arch/arm/mach-omap2/board-rx51-camera.c
> >>>>>>> + *
> >>>>>>> + * Copyright (C) 2008 Nokia Corporation
> >>>>>>> + *
> >>>>>>> + * Contact: Sakari Ailus <[email protected]>
> >>>>>>> + * Tuukka Toivonen <[email protected]>
> >>>>>>
> >>>>>> You should put these people to CC... Just to see if the addresses are
> >>>>>> still valid (which I doubt).
> >>>>>
> >>>>> Ok, trying :-)
> >>>>
> >>>> I got "Delivery Status Notification (Failure)" for both addresses.
> >>
> >> This is expected.
> >>
> >>> Sakari Ailus hosts some code on github [0], which has the following
> >>> email address:
> >>> [email protected]
> >>>
> >>> I added it to this mail's CC.
> >>>
> >>> [0] https://gitorious.org/~sailus
> >>
> >> Nice to hear people are interested in this. ;-)
> >>
> >> The primary reason I haven't tried submitting this to mainline is that
> >> ARM board code has a bad reputation these days. The N900 does not have
> >> yet support for device tree (AFAIK), which also would require a few bits
> >> and pieces on the flash driver to work.
> >>
> >> Also the sensor and lens drivers would need at least some work before
> >> being ready for submission to mainline for camera to be usable.
> >> Unfortunately I haven't had recently time to work on this. N9(50) support
> >> has higher priority for myself. That, too, is pending the DT support for
> >> the device.
> >>
> >> There's indeed more up-to-date code in my repository. Even if it's not
> >> too close to mainline anymore it should be a better starting point than
> >> the old kernel from MeeGo.
> >>
> >> <URL:https://gitorious.org/omap3camera/pages/Home>
> >
> > Hi,
> >
> > this board code is same in your git repository and on meego obs.
> >
> > Patch only adding support for adp1653 driver which is already in upstream
> > kernel.
> >
> > Are there any other problems with this patch to go for upstream?
>
> A few more things comes to mind. We depend a little bit on actual board
> code; it's not only static data. That's the configuration of the external
> clock from the ISP to the sensor. That should move to the common clock
> framework so that the ISP registers the clock and the sensor driver can then
> use it. AFAIR Laurent has done some work on that.

Yes. I hope to get the patches in v3.10.

> The peculiar detail of the rx51 is that there's a switch on the camera CCP2
> bus that selects either sensor (primary or secondary). Both sensors are
> connected to the same receiver. That isn't properly modelled currently at
> all, so that's why we have rx51_camera_set_xshutdown(). I guess it'd still
> work if you only power (i.e. open) either of the devices at a time, though.

Have you thought about how we could model that ?

> That should be all.

--
Regards,

Laurent Pinchart

2013-04-03 22:22:35

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

Hi Laurent,

On Tue, Mar 26, 2013 at 12:07:01AM +0100, Laurent Pinchart wrote:
> On Sunday 24 March 2013 23:46:01 Sakari Ailus wrote:
> > Pali Roh?r wrote:
> > > On Thursday 07 March 2013 23:18:27 Sakari Ailus wrote:
> > >> On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian Reichel wrote:
> > >>> On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Roh?r wrote:
> > >>>> On Wednesday 06 March 2013 21:12:06 Pali Roh?r wrote:
> > >>>>> On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> > >>>>>> On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Roh?r wrote:
> > >>>>>>> +/*
> > >>>>>>> + * arch/arm/mach-omap2/board-rx51-camera.c
> > >>>>>>> + *
> > >>>>>>> + * Copyright (C) 2008 Nokia Corporation
> > >>>>>>> + *
> > >>>>>>> + * Contact: Sakari Ailus <[email protected]>
> > >>>>>>> + * Tuukka Toivonen <[email protected]>
> > >>>>>>
> > >>>>>> You should put these people to CC... Just to see if the addresses are
> > >>>>>> still valid (which I doubt).
> > >>>>>
> > >>>>> Ok, trying :-)
> > >>>>
> > >>>> I got "Delivery Status Notification (Failure)" for both addresses.
> > >>
> > >> This is expected.
> > >>
> > >>> Sakari Ailus hosts some code on github [0], which has the following
> > >>> email address:
> > >>> [email protected]
> > >>>
> > >>> I added it to this mail's CC.
> > >>>
> > >>> [0] https://gitorious.org/~sailus
> > >>
> > >> Nice to hear people are interested in this. ;-)
> > >>
> > >> The primary reason I haven't tried submitting this to mainline is that
> > >> ARM board code has a bad reputation these days. The N900 does not have
> > >> yet support for device tree (AFAIK), which also would require a few bits
> > >> and pieces on the flash driver to work.
> > >>
> > >> Also the sensor and lens drivers would need at least some work before
> > >> being ready for submission to mainline for camera to be usable.
> > >> Unfortunately I haven't had recently time to work on this. N9(50) support
> > >> has higher priority for myself. That, too, is pending the DT support for
> > >> the device.
> > >>
> > >> There's indeed more up-to-date code in my repository. Even if it's not
> > >> too close to mainline anymore it should be a better starting point than
> > >> the old kernel from MeeGo.
> > >>
> > >> <URL:https://gitorious.org/omap3camera/pages/Home>
> > >
> > > Hi,
> > >
> > > this board code is same in your git repository and on meego obs.
> > >
> > > Patch only adding support for adp1653 driver which is already in upstream
> > > kernel.
> > >
> > > Are there any other problems with this patch to go for upstream?
> >
> > A few more things comes to mind. We depend a little bit on actual board
> > code; it's not only static data. That's the configuration of the external
> > clock from the ISP to the sensor. That should move to the common clock
> > framework so that the ISP registers the clock and the sensor driver can then
> > use it. AFAIR Laurent has done some work on that.
>
> Yes. I hope to get the patches in v3.10.

Cool! :)

> > The peculiar detail of the rx51 is that there's a switch on the camera CCP2
> > bus that selects either sensor (primary or secondary). Both sensors are
> > connected to the same receiver. That isn't properly modelled currently at
> > all, so that's why we have rx51_camera_set_xshutdown(). I guess it'd still
> > work if you only power (i.e. open) either of the devices at a time, though.
>
> Have you thought about how we could model that ?

Well, the two dependent gpios could be modelled as two independent ones (
for sensor drivers), but setting the state of those gpios could fail,
gpio_set_value() still returns void. This isn't pretty perhaps but as a
result the initialisation of the secondary sensor to be powered up at the
same time will fail since it's in reset: the xshutdown of both sensors is
controlled by the same gpio as is the mux (AFAIR).

So one N900 camera specific gpio driver would be needed. It'd be a very
simple driver. What do you think?

--
Cheers,

Sakari Ailus
e-mail: [email protected] XMPP: [email protected]

2013-04-04 13:10:29

by Laurent Pinchart

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

Hi Sakari,

On Thursday 04 April 2013 01:22:28 Sakari Ailus wrote:
> On Tue, Mar 26, 2013 at 12:07:01AM +0100, Laurent Pinchart wrote:
> > On Sunday 24 March 2013 23:46:01 Sakari Ailus wrote:
> > > Pali Roh?r wrote:
> > > > On Thursday 07 March 2013 23:18:27 Sakari Ailus wrote:
> > > >> On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian Reichel wrote:
> > > >>> On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Roh?r wrote:
> > > >>>> On Wednesday 06 March 2013 21:12:06 Pali Roh?r wrote:
> > > >>>>> On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> > > >>>>>> On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Roh?r wrote:
> > > >>>>>>> +/*
> > > >>>>>>> + * arch/arm/mach-omap2/board-rx51-camera.c
> > > >>>>>>> + *
> > > >>>>>>> + * Copyright (C) 2008 Nokia Corporation
> > > >>>>>>> + *
> > > >>>>>>> + * Contact: Sakari Ailus <[email protected]>
> > > >>>>>>> + * Tuukka Toivonen <[email protected]>
> > > >>>>>>
> > > >>>>>> You should put these people to CC... Just to see if the addresses
> > > >>>>>> are still valid (which I doubt).
> > > >>>>>
> > > >>>>> Ok, trying :-)
> > > >>>>
> > > >>>> I got "Delivery Status Notification (Failure)" for both addresses.
> > > >>
> > > >> This is expected.
> > > >>
> > > >>> Sakari Ailus hosts some code on github [0], which has the following
> > > >>> email address:
> > > >>> [email protected]
> > > >>>
> > > >>> I added it to this mail's CC.
> > > >>>
> > > >>> [0] https://gitorious.org/~sailus
> > > >>
> > > >> Nice to hear people are interested in this. ;-)
> > > >>
> > > >> The primary reason I haven't tried submitting this to mainline is
> > > >> that ARM board code has a bad reputation these days. The N900 does
> > > >> not have yet support for device tree (AFAIK), which also would
> > > >> require a few bits and pieces on the flash driver to work.
> > > >>
> > > >> Also the sensor and lens drivers would need at least some work before
> > > >> being ready for submission to mainline for camera to be usable.
> > > >> Unfortunately I haven't had recently time to work on this. N9(50)
> > > >> support has higher priority for myself. That, too, is pending the DT
> > > >> support for the device.
> > > >>
> > > >> There's indeed more up-to-date code in my repository. Even if it's
> > > >> not too close to mainline anymore it should be a better starting
> > > >> point than the old kernel from MeeGo.
> > > >>
> > > >> <URL:https://gitorious.org/omap3camera/pages/Home>
> > > >
> > > > Hi,
> > > >
> > > > this board code is same in your git repository and on meego obs.
> > > >
> > > > Patch only adding support for adp1653 driver which is already in
> > > > upstream kernel.
> > > >
> > > > Are there any other problems with this patch to go for upstream?
> > >
> > > A few more things comes to mind. We depend a little bit on actual board
> > > code; it's not only static data. That's the configuration of the
> > > external clock from the ISP to the sensor. That should move to the
> > > common clock framework so that the ISP registers the clock and the
> > > sensor driver can then use it. AFAIR Laurent has done some work on that.
> >
> > Yes. I hope to get the patches in v3.10.
>
> Cool! :)

The patches have been posted to the linux-media mailing list. If the
dependencies make it to v3.10 the OMAP3 ISP patches should get there too.

> > > The peculiar detail of the rx51 is that there's a switch on the camera
> > > CCP2 bus that selects either sensor (primary or secondary). Both sensors
> > > are connected to the same receiver. That isn't properly modelled
> > > currently at all, so that's why we have rx51_camera_set_xshutdown(). I
> > > guess it'd still work if you only power (i.e. open) either of the
> > > devices at a time, though.
> >
> > Have you thought about how we could model that ?
>
> Well, the two dependent gpios could be modelled as two independent ones (
> for sensor drivers), but setting the state of those gpios could fail,
> gpio_set_value() still returns void. This isn't pretty perhaps but as a
> result the initialisation of the secondary sensor to be powered up at the
> same time will fail since it's in reset: the xshutdown of both sensors is
> controlled by the same gpio as is the mux (AFAIR).
>
> So one N900 camera specific gpio driver would be needed. It'd be a very
> simple driver. What do you think?

I think I'll need to see how the GPIOs are wired up on the board.

--
Regards,

Laurent Pinchart

2013-09-06 20:34:13

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

On Wednesday 06 March 2013 21:12:06 Pali Rohár wrote:
> On Sunday 17 February 2013 20:03:03 Aaro Koskinen wrote:
> > Hi,
> >
> > On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali Rohár wrote:
> > > I'm sending ADP1653 flash torch board code for Nokia
> > > RX-51. Kernel driver ADP1653 is already in upstream
> > > kernel. Board code was extracted from this big camera
> > > meego patch:
> > >
> > > https://api.pub.meego.com/public/source/CE:Adaptation:N900
> > > /k
> > > ernel-adaptation-n900/linux-2.6-Camera-for-Meego-N900-Ada
> > > pta tion-kernel-2.6.37-patch.patch
> >
> > You need to sign-off the patch.
>
> Signed-off-by: Pali Rohár <[email protected]>
>
> > > --- /dev/null
> > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> >
> > I'm not sure if adding a new file is sensible. There are
> > already 3 board files for RX-51, which I think is overkill.
>
> You can see that camera board code is big, so code was moved
> to separate file. Because not all camera drivers are
> upstreamed yet there is no camera support in RX-51 board
> code. Current peripheral file for RX-51 is big too and split
> camera code into separate file can be usefull...
>
> > > @@ -0,0 +1,177 @@
> > > +/*
> > > + * arch/arm/mach-omap2/board-rx51-camera.c
> > > + *
> > > + * Copyright (C) 2008 Nokia Corporation
> > > + *
> > > + * Contact: Sakari Ailus <[email protected]>
> > > + * Tuukka Toivonen <[email protected]>
> >
> > You should put these people to CC... Just to see if the
> > addresses are still valid (which I doubt).
>
> Ok, trying :-)
>
> > > +static int __init rx51_adp1653_init(void)
> > > +{
> > > + int err;
> > > +
> > > + err = gpio_request(ADP1653_GPIO_ENABLE, "adp1653
> > > enable"); + if (err) {
> > > + printk(KERN_ERR ADP1653_NAME
> > > + " Failed to request EN gpio\n");
> > > + err = -ENODEV;
> > > + goto err_omap_request_gpio;
> > > + }
> > > +
> > > + err = gpio_request(ADP1653_GPIO_INT, "adp1653
> > > interrupt"); + if (err) {
> > > + printk(KERN_ERR ADP1653_NAME " Failed to request IRQ
> > > gpio\n"); + err = -ENODEV;
> > > + goto err_omap_request_gpio_2;
> > > + }
> > > +
> > > + err = gpio_request(ADP1653_GPIO_STROBE, "adp1653
> > > strobe"); + if (err) {
> > > + printk(KERN_ERR ADP1653_NAME
> > > + " Failed to request STROBE gpio\n");
> > > + err = -ENODEV;
> > > + goto err_omap_request_gpio_3;
> > > + }
> > > +
> > > + gpio_direction_output(ADP1653_GPIO_ENABLE, 0);
> > > + gpio_direction_input(ADP1653_GPIO_INT);
> > > + gpio_direction_output(ADP1653_GPIO_STROBE, 0);
> >
> > gpio_request_array() should be used.
>
> Ok, fixing this.
>
> > > +void __init rx51_camera_init(void)
> > > +{
> > > + if (rx51_camera_hw_init()) {
> > > + printk(KERN_WARNING "%s: Unable to initialize
> > > camera\n", + __func__);
> > > + return;
> > > + }
> > > +
> > > + if (omap3_init_camera(&rx51_isp_platform_data) < 0)
> > > + printk(KERN_WARNING "%s: Unable to register camera
> > > platform " + "device\n", __func__);
> >
> > pr_warn() should be used.
> >
> > A.
>
> Fixed too.
>
> Here is new version v2 of this patch:
>
> diff --git a/arch/arm/mach-omap2/Kconfig
> b/arch/arm/mach-omap2/Kconfig index 41b581f..268fa57 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -287,6 +287,7 @@ config MACH_NOKIA_RX51
> depends on ARCH_OMAP3
> default y
> select OMAP_PACKAGE_CBB
> + select VIDEO_ADP1653 if VIDEO_OMAP3 &&
> VIDEO_HELPER_CHIPS_AUTO
>
> config MACH_OMAP_ZOOM2
> bool "OMAP3 Zoom2 board"
> diff --git a/arch/arm/mach-omap2/Makefile
> b/arch/arm/mach-omap2/Makefile index 947cafe..f20f693 100644
> --- a/arch/arm/mach-omap2/Makefile
> +++ b/arch/arm/mach-omap2/Makefile
> @@ -236,6 +236,7 @@ obj-$(CONFIG_MACH_NOKIA_RM680) +=
> board-rm680.o sdram-nokia.o obj-$(CONFIG_MACH_NOKIA_RX51) +=
> board-rx51.o sdram-nokia.o obj-$(CONFIG_MACH_NOKIA_RX51)
+=
> board-rx51-peripherals.o obj-$(CONFIG_MACH_NOKIA_RX51) +=
> board-rx51-video.o +obj-$(CONFIG_MACH_NOKIA_RX51) +=
> board-rx51-camera.o obj-$(CONFIG_MACH_OMAP_ZOOM2) +=
> board-zoom.o board-zoom-peripherals.o
> obj-$(CONFIG_MACH_OMAP_ZOOM2) += board-zoom-display.o
> obj-$(CONFIG_MACH_OMAP_ZOOM2) += board-zoom-debugboard.o
> diff --git a/arch/arm/mach-omap2/board-rx51-camera.c
> b/arch/arm/mach-omap2/board-rx51-camera.c new file mode
> 100644
> index 0000000..80057ab
> --- /dev/null
> +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> @@ -0,0 +1,152 @@
> +/*
> + * arch/arm/mach-omap2/board-rx51-camera.c
> + *
> + * Copyright (C) 2008 Nokia Corporation
> + *
> + * Contact: Sakari Ailus <[email protected]>
> + * Tuukka Toivonen <[email protected]>
> + *
> + * This program is free software; you can redistribute it
> and/or + * modify it under the terms of the GNU General
> Public License + * version 2 as published by the Free
> Software Foundation. + *
> + * This program is distributed in the hope that it will be
> useful, but + * WITHOUT ANY WARRANTY; without even the
> implied warranty of + * MERCHANTABILITY or FITNESS FOR A
> PARTICULAR PURPOSE. See the GNU + * General Public License
> for more details.
> + *
> + * You should have received a copy of the GNU General Public
> License + * along with this program; if not, write to the
> Free Software + * Foundation, Inc., 51 Franklin St, Fifth
> Floor, Boston, MA + * 02110-1301 USA
> + *
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +
> +#include "../../../drivers/media/platform/omap3isp/isp.h"
> +
> +#include <media/adp1653.h>
> +
> +#include "devices.h"
> +
> +#define ADP1653_GPIO_ENABLE 88 /* Used for resetting ADP1653
> */ +#define ADP1653_GPIO_INT 167 /* Fault interrupt */
> +#define ADP1653_GPIO_STROBE 126 /* Pin used in cam_strobe
> mode -> + * control using ISP drivers */
> +
> +static struct gpio rx51_adp1653_gpios[] __initdata = {
> + { ADP1653_GPIO_ENABLE, GPIOF_OUT_INIT_LOW, "adp1653
enable"
> }, + { ADP1653_GPIO_INT, GPIOF_IN, "adp1653 interrupt" },
> + { ADP1653_GPIO_STROBE, GPIOF_OUT_INIT_LOW, "adp1653
strobe"
> }, +};
> +
> +static int __init rx51_adp1653_init(void)
> +{
> + int ret;
> +
> + ret = gpio_request_array(rx51_adp1653_gpios,
> + ARRAY_SIZE(rx51_adp1653_gpios));
> + if (ret < 0) {
> + pr_err(ADP1653_NAME ": Failed to request gpios\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> +static int __init rx51_camera_hw_init(void)
> +{
> + int rval;
> +
> + rval = rx51_adp1653_init();
> + if (rval)
> + return rval;
> +
> + return 0;
> +}
> +
> +/*
> + *
> + * ADP1653
> + *
> + */
> +
> +static int rx51_adp1653_power(struct v4l2_subdev *subdev, int
> on) +{
> + gpio_set_value(ADP1653_GPIO_ENABLE, on);
> + if (on) {
> + /* Some delay is apparently required. */
> + udelay(20);
> + }
> +
> + return 0;
> +}
> +
> +static struct adp1653_platform_data
> rx51_adp1653_platform_data = { + .power =
> rx51_adp1653_power,
> + /* Must be limited to 500 ms in RX-51 */
> + .max_flash_timeout = 500000, /* us */
> + /* Must be limited to 320 mA in RX-51 B3 and newer hardware
> */ + .max_flash_intensity =
> ADP1653_FLASH_INTENSITY_REG_TO_mA(19), + /* Must be limited
> to 50 mA in RX-51 */
> + .max_torch_intensity =
> ADP1653_FLASH_INTENSITY_REG_TO_mA(1),
> + .max_indicator_intensity =
> ADP1653_INDICATOR_INTENSITY_REG_TO_uA(
> + ADP1653_REG_OUT_SEL_ILED_MAX),
> +};
> +
> +/*
> + *
> + * Init it all
> + *
> + */
> +
> +#define ADP1653_I2C_BUS_NUM 2
> +
> +static struct i2c_board_info rx51_camera_i2c_devices[] = {
> + {
> + I2C_BOARD_INFO(ADP1653_NAME, ADP1653_I2C_ADDR),
> + .platform_data = &rx51_adp1653_platform_data,
> + },
> +};
> +
> +static struct isp_subdev_i2c_board_info
> rx51_camera_primary_subdevs[] = { + {
> + .board_info = &rx51_camera_i2c_devices[0],
> + .i2c_adapter_id = ADP1653_I2C_BUS_NUM,
> + },
> + { NULL, 0, },
> +};
> +
> +static struct isp_v4l2_subdevs_group rx51_camera_subdevs[] =
> { + {
> + .subdevs = rx51_camera_primary_subdevs,
> + .interface = ISP_INTERFACE_CCP2B_PHY1,
> + .bus = { .ccp2 = {
> + .strobe_clk_pol = 0,
> + .crc = 1,
> + .ccp2_mode = 1,
> + .phy_layer = 1,
> + .vpclk_div = 1,
> + } },
> + },
> + { NULL, 0, },
> +};
> +
> +static struct isp_platform_data rx51_isp_platform_data = {
> + .subdevs = rx51_camera_subdevs,
> +};
> +
> +void __init rx51_camera_init(void)
> +{
> + if (rx51_camera_hw_init()) {
> + pr_warn("%s: Unable to initialize camera\n", __func__);
> + return;
> + }
> +
> + if (omap3_init_camera(&rx51_isp_platform_data) < 0)
> + pr_warn("%s: Unable to register camera platform "
> + "device\n", __func__);
> +}
> diff --git a/arch/arm/mach-omap2/board-rx51.c
> b/arch/arm/mach-omap2/board-rx51.c index d0374ea..92117a13
> 100644
> --- a/arch/arm/mach-omap2/board-rx51.c
> +++ b/arch/arm/mach-omap2/board-rx51.c
> @@ -75,6 +75,7 @@ static struct platform_device leds_gpio = {
> */
>
> extern void __init rx51_peripherals_init(void);
> +extern void __init rx51_camera_init(void);
>
> #ifdef CONFIG_OMAP_MUX
> static struct omap_board_mux board_mux[] __initdata = {
> @@ -100,6 +101,7 @@ static void __init rx51_init(void)
>
> usb_musb_init(&musb_board_data);
> rx51_peripherals_init();
> + rx51_camera_init();
>
> /* Ensure SDRC pins are mux'd for self-refresh */
> omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);

Ping, can you review this patch v2?

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2013-09-06 20:35:24

[permalink] [raw]

Subject: Re: [PATCH v2] Camera drivers for Nokia RX-51

On Thursday 04 April 2013 15:11:27 Laurent Pinchart wrote:
> Hi Sakari,
>
> On Thursday 04 April 2013 01:22:28 Sakari Ailus wrote:
> > On Tue, Mar 26, 2013 at 12:07:01AM +0100, Laurent Pinchart
wrote:
> > > On Sunday 24 March 2013 23:46:01 Sakari Ailus wrote:
> > > > Pali Rohár wrote:
> > > > > On Thursday 07 March 2013 23:18:27 Sakari Ailus wrote:
> > > > >> On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian
Reichel wrote:
> > > > >>> On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Rohár
wrote:
> > > > >>>> On Wednesday 06 March 2013 21:12:06 Pali Rohár
wrote:
> > > > >>>>> On Sunday 17 February 2013 20:03:03 Aaro Koskinen
wrote:
> > > > >>>>>> On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali
Rohár wrote:
> > > > >>>>>>> +/*
> > > > >>>>>>> + * arch/arm/mach-omap2/board-rx51-camera.c
> > > > >>>>>>> + *
> > > > >>>>>>> + * Copyright (C) 2008 Nokia Corporation
> > > > >>>>>>> + *
> > > > >>>>>>> + * Contact: Sakari Ailus
> > > > >>>>>>> <[email protected]> + * Tuukka
> > > > >>>>>>> Toivonen <[email protected]>
> > > > >>>>>>
> > > > >>>>>> You should put these people to CC... Just to see
> > > > >>>>>> if the addresses are still valid (which I
> > > > >>>>>> doubt).
> > > > >>>>>
> > > > >>>>> Ok, trying :-)
> > > > >>>>
> > > > >>>> I got "Delivery Status Notification (Failure)" for
> > > > >>>> both addresses.
> > > > >>
> > > > >> This is expected.
> > > > >>
> > > > >>> Sakari Ailus hosts some code on github [0], which
> > > > >>> has the following email address:
> > > > >>> [email protected]
> > > > >>>
> > > > >>> I added it to this mail's CC.
> > > > >>>
> > > > >>> [0] https://gitorious.org/~sailus
> > > > >>
> > > > >> Nice to hear people are interested in this. ;-)
> > > > >>
> > > > >> The primary reason I haven't tried submitting this to
> > > > >> mainline is that ARM board code has a bad reputation
> > > > >> these days. The N900 does not have yet support for
> > > > >> device tree (AFAIK), which also would require a few
> > > > >> bits and pieces on the flash driver to work.
> > > > >>
> > > > >> Also the sensor and lens drivers would need at least
> > > > >> some work before being ready for submission to
> > > > >> mainline for camera to be usable. Unfortunately I
> > > > >> haven't had recently time to work on this. N9(50)
> > > > >> support has higher priority for myself. That, too,
> > > > >> is pending the DT support for the device.
> > > > >>
> > > > >> There's indeed more up-to-date code in my repository.
> > > > >> Even if it's not too close to mainline anymore it
> > > > >> should be a better starting point than the old
> > > > >> kernel from MeeGo.
> > > > >>
> > > > >> <URL:https://gitorious.org/omap3camera/pages/Home>
> > > > >
> > > > > Hi,
> > > > >
> > > > > this board code is same in your git repository and on
> > > > > meego obs.
> > > > >
> > > > > Patch only adding support for adp1653 driver which is
> > > > > already in upstream kernel.
> > > > >
> > > > > Are there any other problems with this patch to go for
> > > > > upstream?
> > > >
> > > > A few more things comes to mind. We depend a little bit
> > > > on actual board code; it's not only static data. That's
> > > > the configuration of the external clock from the ISP to
> > > > the sensor. That should move to the common clock
> > > > framework so that the ISP registers the clock and the
> > > > sensor driver can then use it. AFAIR Laurent has done
> > > > some work on that.
> > >
> > > Yes. I hope to get the patches in v3.10.
> >
> > Cool! :)
>
> The patches have been posted to the linux-media mailing list.
> If the dependencies make it to v3.10 the OMAP3 ISP patches
> should get there too.
>
> > > > The peculiar detail of the rx51 is that there's a switch
> > > > on the camera CCP2 bus that selects either sensor
> > > > (primary or secondary). Both sensors are connected to
> > > > the same receiver. That isn't properly modelled
> > > > currently at all, so that's why we have
> > > > rx51_camera_set_xshutdown(). I guess it'd still work if
> > > > you only power (i.e. open) either of the devices at a
> > > > time, though.
> > >
> > > Have you thought about how we could model that ?
> >
> > Well, the two dependent gpios could be modelled as two
> > independent ones ( for sensor drivers), but setting the
> > state of those gpios could fail, gpio_set_value() still
> > returns void. This isn't pretty perhaps but as a result the
> > initialisation of the secondary sensor to be powered up at
> > the same time will fail since it's in reset: the xshutdown
> > of both sensors is controlled by the same gpio as is the
> > mux (AFAIR).
> >
> > So one N900 camera specific gpio driver would be needed.
> > It'd be a very simple driver. What do you think?
>
> I think I'll need to see how the GPIOs are wired up on the
> board.

Hello, after months, what is state of drivers now?

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2013-09-07 23:03:04

by Aaro Koskinen

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

Hi,

On Fri, Sep 06, 2013 at 10:34:05PM +0200, Pali Roh?r wrote:
> > --- /dev/null
> > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
[...]
> Ping, can you review this patch v2?

I don't think Tony will accept any new board stuff for RX-51/N900.
See for example: http://marc.info/?l=linux-kernel&m=137629626213187&w=2

There should be initial Nokia N900 DTS file in 3.12-rc1, and we should
continue converting this board fully to DT.

A.

2013-09-09 21:41:07

[permalink] [raw]

Subject: Re: [PATCH v2] Camera drivers for Nokia RX-51

Hi Pali,

On Fri, Sep 06, 2013 at 10:35:18PM +0200, Pali Roh?r wrote:
> On Thursday 04 April 2013 15:11:27 Laurent Pinchart wrote:
> > Hi Sakari,
> >
> > On Thursday 04 April 2013 01:22:28 Sakari Ailus wrote:
> > > On Tue, Mar 26, 2013 at 12:07:01AM +0100, Laurent Pinchart
> wrote:
> > > > On Sunday 24 March 2013 23:46:01 Sakari Ailus wrote:
> > > > > Pali Roh?r wrote:
> > > > > > On Thursday 07 March 2013 23:18:27 Sakari Ailus wrote:
> > > > > >> On Wed, Mar 06, 2013 at 10:44:41PM +0100, Sebastian
> Reichel wrote:
> > > > > >>> On Wed, Mar 06, 2013 at 09:20:16PM +0100, Pali Roh?r
> wrote:
> > > > > >>>> On Wednesday 06 March 2013 21:12:06 Pali Roh?r
> wrote:
> > > > > >>>>> On Sunday 17 February 2013 20:03:03 Aaro Koskinen
> wrote:
> > > > > >>>>>> On Sun, Feb 17, 2013 at 04:16:49PM +0100, Pali
> Roh?r wrote:
> > > > > >>>>>>> +/*
> > > > > >>>>>>> + * arch/arm/mach-omap2/board-rx51-camera.c
> > > > > >>>>>>> + *
> > > > > >>>>>>> + * Copyright (C) 2008 Nokia Corporation
> > > > > >>>>>>> + *
> > > > > >>>>>>> + * Contact: Sakari Ailus
> > > > > >>>>>>> <[email protected]> + * Tuukka
> > > > > >>>>>>> Toivonen <[email protected]>
> > > > > >>>>>>
> > > > > >>>>>> You should put these people to CC... Just to see
> > > > > >>>>>> if the addresses are still valid (which I
> > > > > >>>>>> doubt).
> > > > > >>>>>
> > > > > >>>>> Ok, trying :-)
> > > > > >>>>
> > > > > >>>> I got "Delivery Status Notification (Failure)" for
> > > > > >>>> both addresses.
> > > > > >>
> > > > > >> This is expected.
> > > > > >>
> > > > > >>> Sakari Ailus hosts some code on github [0], which
> > > > > >>> has the following email address:
> > > > > >>> [email protected]
> > > > > >>>
> > > > > >>> I added it to this mail's CC.
> > > > > >>>
> > > > > >>> [0] https://gitorious.org/~sailus
> > > > > >>
> > > > > >> Nice to hear people are interested in this. ;-)
> > > > > >>
> > > > > >> The primary reason I haven't tried submitting this to
> > > > > >> mainline is that ARM board code has a bad reputation
> > > > > >> these days. The N900 does not have yet support for
> > > > > >> device tree (AFAIK), which also would require a few
> > > > > >> bits and pieces on the flash driver to work.
> > > > > >>
> > > > > >> Also the sensor and lens drivers would need at least
> > > > > >> some work before being ready for submission to
> > > > > >> mainline for camera to be usable. Unfortunately I
> > > > > >> haven't had recently time to work on this. N9(50)
> > > > > >> support has higher priority for myself. That, too,
> > > > > >> is pending the DT support for the device.
> > > > > >>
> > > > > >> There's indeed more up-to-date code in my repository.
> > > > > >> Even if it's not too close to mainline anymore it
> > > > > >> should be a better starting point than the old
> > > > > >> kernel from MeeGo.
> > > > > >>
> > > > > >> <URL:https://gitorious.org/omap3camera/pages/Home>
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > this board code is same in your git repository and on
> > > > > > meego obs.
> > > > > >
> > > > > > Patch only adding support for adp1653 driver which is
> > > > > > already in upstream kernel.
> > > > > >
> > > > > > Are there any other problems with this patch to go for
> > > > > > upstream?
> > > > >
> > > > > A few more things comes to mind. We depend a little bit
> > > > > on actual board code; it's not only static data. That's
> > > > > the configuration of the external clock from the ISP to
> > > > > the sensor. That should move to the common clock
> > > > > framework so that the ISP registers the clock and the
> > > > > sensor driver can then use it. AFAIR Laurent has done
> > > > > some work on that.
> > > >
> > > > Yes. I hope to get the patches in v3.10.
> > >
> > > Cool! :)
> >
> > The patches have been posted to the linux-media mailing list.
> > If the dependencies make it to v3.10 the OMAP3 ISP patches
> > should get there too.
> >
> > > > > The peculiar detail of the rx51 is that there's a switch
> > > > > on the camera CCP2 bus that selects either sensor
> > > > > (primary or secondary). Both sensors are connected to
> > > > > the same receiver. That isn't properly modelled
> > > > > currently at all, so that's why we have
> > > > > rx51_camera_set_xshutdown(). I guess it'd still work if
> > > > > you only power (i.e. open) either of the devices at a
> > > > > time, though.
> > > >
> > > > Have you thought about how we could model that ?
> > >
> > > Well, the two dependent gpios could be modelled as two
> > > independent ones ( for sensor drivers), but setting the
> > > state of those gpios could fail, gpio_set_value() still
> > > returns void. This isn't pretty perhaps but as a result the
> > > initialisation of the secondary sensor to be powered up at
> > > the same time will fail since it's in reset: the xshutdown
> > > of both sensors is controlled by the same gpio as is the
> > > mux (AFAIR).
> > >
> > > So one N900 camera specific gpio driver would be needed.
> > > It'd be a very simple driver. What do you think?
> >
> > I think I'll need to see how the GPIOs are wired up on the
> > board.
>
> Hello, after months, what is state of drivers now?

I have no choice except to say, much to my regret, that it's not really
better than it was half a year ago. That said, I have not taken that off my
to-do list.

Please keep in mind that the user space interface used by the omap3camd (and
implemented by omap34xxcam) is entirely unsupported in the mainline kernel.
A wrapper would need to be implemented to mimic the old interface to that
binary blob --- all the equivalent functionality is there, albeit much
better but quite different. Or it could be re-implemented.

Having working drivers is a pre-dependency to that; I agree.

--
Kind regards,

Sakari Ailus
e-mail: [email protected] XMPP: [email protected]

2013-09-17 23:50:33

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

* Aaro Koskinen <[email protected]> [130907 16:10]:
> Hi,
>
> On Fri, Sep 06, 2013 at 10:34:05PM +0200, Pali Rohár wrote:
> > > --- /dev/null
> > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> [...]
> > Ping, can you review this patch v2?
>
> I don't think Tony will accept any new board stuff for RX-51/N900.
> See for example: http://marc.info/?l=linux-kernel&m=137629626213187&w=2
>
> There should be initial Nokia N900 DTS file in 3.12-rc1, and we should
> continue converting this board fully to DT.

Yes let's plan on making mach-omap2 to be DT only soonish.
AFAIK the only stopper right now for omap3 are the pending
pinctrl changes for the wake-up events as otherwise we would
have sever PM regressions with off-idle.

Regards,

Tony

2013-09-18 13:16:46

by Pavel Machek

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

On Sun 2013-09-08 02:02:52, Aaro Koskinen wrote:
> Hi,
>
> On Fri, Sep 06, 2013 at 10:34:05PM +0200, Pali Roh?r wrote:
> > > --- /dev/null
> > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> [...]
> > Ping, can you review this patch v2?
>
> I don't think Tony will accept any new board stuff for RX-51/N900.
> See for example: http://marc.info/?l=linux-kernel&m=137629626213187&w=2
>
> There should be initial Nokia N900 DTS file in 3.12-rc1, and we should
> continue converting this board fully to DT.

That's not reasonable. N900 is pretty far away from working with
device tree. Yes, I was able to get some bits merged, but that should
not be excuse to halt other N900 development.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2013-09-18 14:12:29

by Javier Martinez Canillas

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

On Wed, Sep 18, 2013 at 1:50 AM, Tony Lindgren <[email protected]> wrote:
> * Aaro Koskinen <[email protected]> [130907 16:10]:
>> Hi,
>>
>> On Fri, Sep 06, 2013 at 10:34:05PM +0200, Pali Rohár wrote:
>> > > --- /dev/null
>> > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
>> [...]
>> > Ping, can you review this patch v2?
>>
>> I don't think Tony will accept any new board stuff for RX-51/N900.
>> See for example: http://marc.info/?l=linux-kernel&m=137629626213187&w=2
>>
>> There should be initial Nokia N900 DTS file in 3.12-rc1, and we should
>> continue converting this board fully to DT.
>
> Yes let's plan on making mach-omap2 to be DT only soonish.
> AFAIK the only stopper right now for omap3 are the pending
> pinctrl changes for the wake-up events as otherwise we would
> have sever PM regressions with off-idle.
>

Hi Tony,

I don't know if OMAP2+ DT will happen soon as you said. At least I
know about a big issue we had with GPIO pins not being auto-requested
when are mapped as IRQ. You can refer to [1] for the latest approach
and how this discussion have been going.

This is something that we have been trying to solve for a couple of
kernel releases by now so it would be great if you can join the
discussion.

> Regards,
>
> Tony

Thanks a lot and best regards,
Javier

[1]: https://lkml.org/lkml/2013/8/26/260

2013-09-18 16:00:16

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

On Wednesday 18 September 2013 15:16:44 Pavel Machek wrote:
> On Sun 2013-09-08 02:02:52, Aaro Koskinen wrote:
> > Hi,
> >
> > On Fri, Sep 06, 2013 at 10:34:05PM +0200, Pali Rohár wrote:
> > > > --- /dev/null
> > > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> >
> > [...]
> >
> > > Ping, can you review this patch v2?
> >
> > I don't think Tony will accept any new board stuff for
> > RX-51/N900. See for example:
> > http://marc.info/?l=linux-kernel&m=137629626213187&w=2
> >
> > There should be initial Nokia N900 DTS file in 3.12-rc1, and
> > we should continue converting this board fully to DT.
>
> That's not reasonable. N900 is pretty far away from working
> with device tree. Yes, I was able to get some bits merged,
> but that should not be excuse to halt other N900 development.
> Pavel

Right. So Tony, will you accept future patches for board files?

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2013-09-18 17:42:18

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

* Pali Rohár <[email protected]> [130918 09:08]:
> On Wednesday 18 September 2013 15:16:44 Pavel Machek wrote:
> > On Sun 2013-09-08 02:02:52, Aaro Koskinen wrote:
> > > Hi,
> > >
> > > On Fri, Sep 06, 2013 at 10:34:05PM +0200, Pali Rohár wrote:
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> > >
> > > [...]
> > >
> > > > Ping, can you review this patch v2?
> > >
> > > I don't think Tony will accept any new board stuff for
> > > RX-51/N900. See for example:
> > > http://marc.info/?l=linux-kernel&m=137629626213187&w=2
> > >
> > > There should be initial Nokia N900 DTS file in 3.12-rc1, and
> > > we should continue converting this board fully to DT.
> >
> > That's not reasonable. N900 is pretty far away from working
> > with device tree. Yes, I was able to get some bits merged,
> > but that should not be excuse to halt other N900 development.
> > Pavel
>
> Right. So Tony, will you accept future patches for board files?

Only fixes to board-*.c files please unless there's a _really_
good reason to make things more complex with the platform data.
Let's not make the DT conversion any more complex than it
already is.

Getting things working with DT for the devices is pretty trivial
in most cases. And we can also still initialize the legacy devices
using platform data the same way as earlier.

Regards,

Tony

2013-09-18 17:45:26

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

* Javier Martinez Canillas <[email protected]> [130918 07:20]:
>
> Hi Tony,
>
> I don't know if OMAP2+ DT will happen soon as you said. At least I
> know about a big issue we had with GPIO pins not being auto-requested
> when are mapped as IRQ. You can refer to [1] for the latest approach
> and how this discussion have been going.
>
> This is something that we have been trying to solve for a couple of
> kernel releases by now so it would be great if you can join the
> discussion.

Yes I'm painfully aware of that mess too, that needs to be
sorted out for sure.

Regards,

Tony

> [1]: https://lkml.org/lkml/2013/8/26/260

2013-09-19 22:30:42

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

On Wednesday 18 September 2013 19:42:12 Tony Lindgren wrote:
> * Pali Rohár <[email protected]> [130918 09:08]:
> > On Wednesday 18 September 2013 15:16:44 Pavel Machek wrote:
> > > On Sun 2013-09-08 02:02:52, Aaro Koskinen wrote:
> > > > Hi,
> > > >
> > > > On Fri, Sep 06, 2013 at 10:34:05PM +0200, Pali Rohár
wrote:
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> > > >
> > > > [...]
> > > >
> > > > > Ping, can you review this patch v2?
> > > >
> > > > I don't think Tony will accept any new board stuff for
> > > > RX-51/N900. See for example:
> > > > http://marc.info/?l=linux-kernel&m=137629626213187&w=2
> > > >
> > > > There should be initial Nokia N900 DTS file in 3.12-rc1,
> > > > and we should continue converting this board fully to
> > > > DT.
> > >
> > > That's not reasonable. N900 is pretty far away from
> > > working with device tree. Yes, I was able to get some
> > > bits merged, but that should not be excuse to halt other
> > > N900 development.
> > >
> > > Pavel
> >
> > Right. So Tony, will you accept future patches for board
> > files?
>
> Only fixes to board-*.c files please unless there's a _really_
> good reason to make things more complex with the platform
> data. Let's not make the DT conversion any more complex than
> it already is.
>
> Getting things working with DT for the devices is pretty
> trivial in most cases. And we can also still initialize the
> legacy devices using platform data the same way as earlier.
>
> Regards,
>
> Tony

So, what to this with this patch v2 (it was sent 06.03.2013)?
It adding board support for ADP1653 flash torch which is part of
rx51 camera code. Driver itself is already in kernel tree.
Can you comment/review this patch?

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2013-09-20 16:33:25

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

* Pali Rohár <[email protected]> [130919 15:38]:
> On Wednesday 18 September 2013 19:42:12 Tony Lindgren wrote:
> > * Pali Rohár <[email protected]> [130918 09:08]:
> > > On Wednesday 18 September 2013 15:16:44 Pavel Machek wrote:
> > > > On Sun 2013-09-08 02:02:52, Aaro Koskinen wrote:
> > > > > Hi,
> > > > >
> > > > > On Fri, Sep 06, 2013 at 10:34:05PM +0200, Pali Rohár
> wrote:
> > > > > > > --- /dev/null
> > > > > > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> > > > >
> > > > > [...]
> > > > >
> > > > > > Ping, can you review this patch v2?
> > > > >
> > > > > I don't think Tony will accept any new board stuff for
> > > > > RX-51/N900. See for example:
> > > > > http://marc.info/?l=linux-kernel&m=137629626213187&w=2
> > > > >
> > > > > There should be initial Nokia N900 DTS file in 3.12-rc1,
> > > > > and we should continue converting this board fully to
> > > > > DT.
> > > >
> > > > That's not reasonable. N900 is pretty far away from
> > > > working with device tree. Yes, I was able to get some
> > > > bits merged, but that should not be excuse to halt other
> > > > N900 development.
> > > >
> > > > Pavel
> > >
> > > Right. So Tony, will you accept future patches for board
> > > files?
> >
> > Only fixes to board-*.c files please unless there's a _really_
> > good reason to make things more complex with the platform
> > data. Let's not make the DT conversion any more complex than
> > it already is.
> >
> > Getting things working with DT for the devices is pretty
> > trivial in most cases. And we can also still initialize the
> > legacy devices using platform data the same way as earlier.
> >
> > Regards,
> >
> > Tony
>
> So, what to this with this patch v2 (it was sent 06.03.2013)?
> It adding board support for ADP1653 flash torch which is part of
> rx51 camera code. Driver itself is already in kernel tree.
> Can you comment/review this patch?

Yeah I'm not taking any more of new platform data like this,
this all is already very trivial to do with device tree
directly in the driver. Adding this would make the pains of
moving to device tree based booting just more of a pain and
guess who has to deal with all of that.

For the hwrng platform data makes still sense even for the
device tree as we don't have any better way currently to do
the SMC callbacks.

Regards,

Tony

2013-09-21 12:37:35

by Pavel Machek

[permalink] [raw]

Subject: Re: [PATCH v2] ADP1653 board code for Nokia RX-51

Hi!

> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/mach-omap2/board-rx51-camera.c
> > > >
> > > > [...]
> > > >
> > > > > Ping, can you review this patch v2?
> > > >
> > > > I don't think Tony will accept any new board stuff for
> > > > RX-51/N900. See for example:
> > > > http://marc.info/?l=linux-kernel&m=137629626213187&w=2
> > > >
> > > > There should be initial Nokia N900 DTS file in 3.12-rc1, and
> > > > we should continue converting this board fully to DT.
> > >
> > > That's not reasonable. N900 is pretty far away from working
> > > with device tree. Yes, I was able to get some bits merged,
> > > but that should not be excuse to halt other N900 development.
> > > Pavel
> >
> > Right. So Tony, will you accept future patches for board files?
>
> Only fixes to board-*.c files please unless there's a _really_
> good reason to make things more complex with the platform data.
> Let's not make the DT conversion any more complex than it
> already is.
>
> Getting things working with DT for the devices is pretty trivial
> in most cases. And we can also still initialize the legacy devices
> using platform data the same way as earlier.

Well... It was quite a struggle to get video working with device tree
in 3.10, and now it is broken in 3.12-rc1, again.

"Now, here, you see, it takes all the running you can do, to keep in
the same place. If you want to get somewhere else, you must run at
least twice as fast as that! "

Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2013-09-23 17:21:54