* Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> [140613 03:30]:
> Hi Tony,
>
> On Friday 13 June 2014 00:53:25 Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> [140612 23:48]:
> > > On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> > > > 1. They live in separate hardware modules that can be clocked separately
> > >
> > > Actually I don't think that's true. The CSI2 PHY is part of the camera
> > > device, with all its registers but the one above in the camera device
> > > register space. For some weird reason a couple of bits were pushed to the
> > > control module, but that doesn't make the CSI2 PHY itself a separate
> > > device.
> >
> > Yes they are separate. Anything in the system control module is
> > a separate hardware module from the other devices. So in this case
> > the CSI2 PHY is part of the system control module, not the camera
> > module.
>
> Section 8.2.3 ("ISS CSI2 PHY") of the OMAP4460 TRM (revision AA) documents the
> CSI2 PHY is being part of the ISS, with three PHY registers in the ISS
> register space (not counting the PHY interrupt and status bits in several
> other ISS registers) and one register in the system control module register
> space. It's far from clear which power domain(s) is (are) involved.
OK I see. The register in the system control module just contains some
pin and clock related resources for the phy.
> > > > 2. Doing a read-back to flush a posted write in one hardware module most
> > > > likely won't flush the write to other and that can lead into hard to
> > > > find mysterious bugs
> > >
> > > The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
> >
> > Right, but you would have to do readbacks both from the phy register and
> > camera register to ensure writes get written. It's best to keep the
> > logic completely separate especially considering that they can be
> > clocked separately.
> >
> > > > 3. If we ever have a common system control module driver, we need to
> > > > rewrite all the system control module register tinkering in the
> > > > drivers
> > >
> > > Sure, but that's already the case today, as the OMAP4 ISS driver already
> > > accesses the control module register directly. I won't make that worse :-)
> >
> > Well it's in staging for a reason :)
> >
> > > > So it's best to try to use an existing framework for it. That avoids
> > > > tons of pain later on ;)
> > >
> > > I agree, but I don't think the PHY framework would be the right
> > > abstraction. As explained above the CSI2 PHY is part of the OMAP4 ISS, so
> > > modeling its single control module register as a PHY would be a hack.
> >
> > Well that register belongs to the system control module, not the
> > camera module. It's not like the camera IO space is out of registers
> > or something! :)
>
> The PHY has 3 registers in the ISS I/O space and one register in the control
> module I/O space. I have no idea why they've split it that way. The clock
> enable bits are especially "interested", the source clock (CAM_PHY_CTRL_FCLK)
> comes from the ISS as documented in section 8.1.1 ("ISS Integration"), is
> gated by the control module (the gated clock is called CTRLCLK) and then goes
> back to the ISS CSI2 PHY (it's mentioned in the CSI2 PHY "REGISTER1"
> documentation).
>
> > We're already handling similar control module phy cases, see for
> > example drivers/phy/phy-omap-control.c. Maybe you have most of the
> > code already there?
>
> I'm afraid not. For PHYs that are in the system control module that solution
> is perfectly fine, but the CSI2 PHY isn't (or at least not all of it).
>
> I would be fine with writing a separate PHY driver if the PHY was completely
> separate. As the documentation doesn't make it clear which part of the
> hardware belongs to which module, matching the software implementation with an
> unknown hardware implementation would be pretty difficult :-)
Yeah it seems the phy driver would still have to use the pin resources
in the system control module.
> If you have a couple of minutes to spare and can look at the CSI2 PHY
> documentation in the TRM, you might be more successful than me figuring out
> how the hardware is implemented.
Took a look and it seems the phy is split into two parts. So probably
using the syscon mapping for the register in scm are is a good start.
At least then there's some protection from drivers tinkering directly
with the system control modules.
Maybe s ee what drivers/regulator/pbias-regulator.c is doing with
syscon to see if that works? Moving that to some phy driver later on
should be trivial if needed :)
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
