Thanks for the comments David. I'll submit a proper patch later.
Cheers,
Zik
On Fri, Feb 29, 2008 at 11:56 AM, David Brownell <[EMAIL PROTECTED]> wrote:
> On Thursday 28 February 2008, Zik Saleeba wrote:
> > I fixed my other suspend/resume problem - it was unrelated to spi. So
> > restoring the SSP registers is the fix. It's all working correctly for
> > me now.
>
> Good. I can believe that driver hasn't had much suspend/resume testing;
> many platforms don't get such testing, and often not all drivers get
> covered. PXA has had issues where even the reference boards (for folk
> who have them) don't really leverage all the peripherals.
>
>
>
> > I've never submitted a patch before so I'd appreciate a review of this
> > before I submit it properly:
>
> See Documentation/SubmittingPatches. The most obvious bit:
>
>
> > Index: pxa2xx_spi.c
> > ===================================================================
> > --- pxa2xx_spi.c (revision 562)
> > +++ pxa2xx_spi.c (working copy)
>
> Patch format should let it be applied at toplevei with "patch -p1" ...
>
>
> > @@ -102,6 +102,12 @@
>
> I prefer to see patches generated with "diff -p ..." so there's
> more context to review *just the patch* ...
>
>
>
> > u32 int_cr1;
> > u32 clear_sr;
> > u32 mask_sr;
> > +
> > + /* saved state for suspend/resume */
> > + u32 saved_sscr0;
> > + u32 saved_sscr1;
> > + u32 saved_sst0;
> > + u32 saved_sspsp;
> >
> > /* Driver message queue */
> > struct workqueue_struct *workqueue;
> > @@ -1570,11 +1576,16 @@
> > static int pxa2xx_spi_suspend(struct platform_device *pdev, pm_message_t
> state)
> > {
> > struct driver_data *drv_data = platform_get_drvdata(pdev);
> > + void *reg = drv_data->ioaddr;
> > int status = 0;
> >
> > - /* Check all childern for current power state */
>
> You should also generate patches aginst the *CURRENT* kernel,
> which in this case doesn't have that needless loop.
>
>
>
> > + /* Save register state */
> > + drv_data->saved_sscr0 = read_SSCR0(reg);
> > + drv_data->saved_sscr1 = read_SSCR1(reg);
> > + if (drv_data->ssp_type != PXA25x_SSP)
> > + drv_data->saved_sst0 = read_SSTO(reg);
> > + drv_data->saved_sspsp = read_SSPSP(reg);
> > +
> > + /* Check all children for current power state */
> > if (device_for_each_child(&pdev->dev, &state, suspend_devices) !=
> 0) {
>
> I think you have some indentation problems ... looks like you're
> either not using TAB for indent, or added an extra space afterwards.
>
>
>
>
> > dev_warn(&pdev->dev, "suspend aborted\n");
> > return -1;
> > @@ -1592,13 +1604,21 @@
> > static int pxa2xx_spi_resume(struct platform_device *pdev)
> > {
> > struct driver_data *drv_data = platform_get_drvdata(pdev);
> > + void *reg = drv_data->ioaddr;
> > int status = 0;
> >
> > /* Enable the SSP clock */
> > pxa_set_cken(drv_data->master_info->clock_enable, 1);
> >
> > + /* Load saved SSP configuration */
> > + write_SSCR0(0, reg);
> > + write_SSCR1(drv_data->saved_sscr1, reg);
> > + write_SSCR0(drv_data->saved_sscr0, reg);
> > + write_SSITR(0, reg);
> > + if (drv_data->ssp_type != PXA25x_SSP)
> > + write_SSTO(drv_data->saved_sst0, reg);
> > + write_SSPSP(drv_data->saved_sspsp, reg);
> > +
> > /* Start the queue running */
> > status = start_queue(drv_data);
> > if (status != 0) {
> >
> >
>
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
spi-devel-general mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
