On Wed, Dec 02, 2020 at 03:44:47PM +0100, Paul Kocialkowski wrote:
> > > +static int __maybe_unused sun6i_mipi_csi2_suspend(struct device *dev)
> > > +{
> > > + struct sun6i_mipi_csi2_dev *cdev = dev_get_drvdata(dev);
> > > +
> > > + clk_disable_unprepare(cdev->clk_mod);
> > > + clk_disable_unprepare(cdev->clk_bus);
> > > + reset_control_assert(cdev->reset);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int __maybe_unused sun6i_mipi_csi2_resume(struct device *dev)
> > > +{
> > > + struct sun6i_mipi_csi2_dev *cdev = dev_get_drvdata(dev);
> > > + int ret;
> > > +
> > > + ret = reset_control_deassert(cdev->reset);
> > > + if (ret) {
> > > + dev_err(cdev->dev, "failed to deassert reset\n");
> > > + return ret;
> > > + }
> > > +
> > > + ret = clk_prepare_enable(cdev->clk_bus);
> > > + if (ret) {
> > > + dev_err(cdev->dev, "failed to enable bus clock\n");
> > > + goto error_reset;
> > > + }
> > > +
> > > + ret = clk_prepare_enable(cdev->clk_mod);
> > > + if (ret) {
> > > + dev_err(cdev->dev, "failed to enable module clock\n");
> > > + goto error_clk_bus;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +error_clk_bus:
> > > + clk_disable_unprepare(cdev->clk_bus);
> > > +
> > > +error_reset:
> > > + reset_control_assert(cdev->reset);
> > > +
> > > + return ret;
> > > +}
> >
> > I'm guessing you set the __maybe_unused attribute because you're using
> > SET_RUNTIME_PM_OPS, but what would happen if runtime_pm isn't selected?
> > It looks like you don't handle that case.
>
> Indeed, __maybe_unused is because of the conditional definition of
> SET_RUNTIME_PM_OPS. If CONFIG_PM is not selected, then I guess the controller
> wouldn't be powered and wouldn't work. So I should definitely add a Kconfig
> dependency on PM then, right?There's two ways we can do it. What you suggested is one, the other is to have something like our SPI driver to call directly the resume function if there's no runtime pm support. Maxime
signature.asc
Description: PGP signature
