Hi Benoit,
On Wed, Oct 11, 2017 at 01:22:59PM +0000, Benoit Parrot wrote:
> Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote on Wed [2017-Oct-11 11:24:09 +0200]:
> > On Fri, Sep 29, 2017 at 05:27:09PM +0000, Benoit Parrot wrote:
> > > > +static int csi2rx_get_resources(struct csi2rx_priv *csi2rx,
> > > > + struct platform_device *pdev)
> > > > +{
> > > > + struct resource *res;
> > > > + unsigned char i;
> > > > + u32 reg;
> > > > +
> > > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > + csi2rx->base = devm_ioremap_resource(&pdev->dev, res);
> > > > + if (IS_ERR(csi2rx->base))
> > > > + return PTR_ERR(csi2rx->base);
> > > > +
> > > > + csi2rx->sys_clk = devm_clk_get(&pdev->dev, "sys_clk");
> > > > + if (IS_ERR(csi2rx->sys_clk)) {
> > > > + dev_err(&pdev->dev, "Couldn't get sys clock\n");
> > > > + return PTR_ERR(csi2rx->sys_clk);
> > > > + }
> > > > +
> > > > + csi2rx->p_clk = devm_clk_get(&pdev->dev, "p_clk");
> > > > + if (IS_ERR(csi2rx->p_clk)) {
> > > > + dev_err(&pdev->dev, "Couldn't get P clock\n");
> > > > + return PTR_ERR(csi2rx->p_clk);
> > > > + }
> > > > +
> > > > + csi2rx->dphy = devm_phy_optional_get(&pdev->dev, "dphy");
> > > > + if (IS_ERR(csi2rx->dphy)) {
> > > > + dev_err(&pdev->dev, "Couldn't get external D-PHY\n");
> > > > + return PTR_ERR(csi2rx->dphy);
> > > > + }
> > > > +
> > > > + /*
> > > > + * FIXME: Once we'll have external D-PHY support, the check
> > > > + * will need to be removed.
> > > > + */
> > > > + if (csi2rx->dphy) {
> > > > + dev_err(&pdev->dev, "External D-PHY not supported yet\n");
> > > > + return -EINVAL;
> > > > + }
> > >
> > > I understand that in your current environment you do not have a
> > > DPHY. But I am wondering in a real setup where you will have either
> > > an internal or an external DPHY, how are they going to interact with
> > > this driver or vice-versa?
> >
> > It's difficult to give an answer with so little details. How would you
> > choose between those two PHYs? Is there a mux, or should we just power
> > one of the two? If that's the case, is there any use case were we
> > might want to power both? If not, which one should we favor, in which
> > situations?
>
> Oops, I guess I should clarify, in this case I did not mean we would
> have both an internal and an external DPHY. I just meant one or the other.
Ok, my bad :)
> Basically just want to see how you would actually handle a DPHY here
> whether it's internal or external?
>
> For instance, using direct register access from within this driver
> or make use of an separate phy driver...
So internal would be easy, the only internal D-PHY that is supposed to
be integrated is Cadence's, and its registers would be located in the
same memory region, so that driver would handle it.
In the external case, the ideal solution would be to extend the phy
framework to deal with more phy types. CSI would be a good candidate,
but we also have that issue with the DSI patches Boris has been
sending, and other phy types like USB.
The idea would be to extend it (or subclass it) to allow to pass more
configuration data that would allow to implement a D-PHY driver of its
own. That's the long term goal obviously, and we haven't started
working on that. So we might have to settle for in-drivers quirks
short term.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Attachment:
signature.asc
Description: PGP signature
