Hi,
On Wed, Oct 10, 2012 at 10:01 PM, Olof Johansson <olof@xxxxxxxxx> wrote:
> Hi,
>
> On Wed, Oct 10, 2012 at 02:08:31PM +0530, Vasanth Ananthan wrote:
>
>> > > +
>> > > + sataphy@70 {
>> >
>> > sata-phy would be a more conventional name.
>> >
>> > > + compatible = "samsung,i2c-phy";
>> >
>> > i2c-phy? Seems like an odd choice of name. What is this device?
>> >
>>
>> The SATA physical layer controller is both a platform device and a i2c
>> slave device.
>> This compatible string is for the i2c client driver. Hence I have used
>> i2c-phy here.
>
> Ok, but samsung,i2c-phy is much too generic. Maybe something like
> samsung,exynos5-sata-phy (and rename the MMIO-controlled phy controls
> to ...sata-phy-controller below).
>
I'll do so..
>
>> > > + };
>> > > +
>> > > + sata-phy@12170000 {
>> > > + compatible = "samsung,exynos-sata-phy";
>> > > + reg = <0x12170000 0x1ff>;
>> > > + };
>> >
>> > Should have binding too. How does this relate to the i2c device above.
>> >
>>
>> As mentioned earlier SATA physical layer controller is both a platform
>> device and also an i2c slave device.
>> This Node is for the SATA physical layer platform device, the previous node
>> is for i2c slave device.
>> Certain initialization settings done directly and other settings has to be
>> done through i2c.
>
> Wow, that's quite awkward. What needs to be done over i2c? I think I have
> seen use of SATA without touching the i2c side but it might have been for
> a simple setup.
40 bit Interface setting is done through i2c. Internal setting address
0x3A and data 0x0B.
>
>> > > +
>> > > i2c@12C60000 {
>> > > compatible = "samsung,s3c2440-i2c";
>> > > reg = <0x12C60000 0x100>;
>> > > @@ -152,6 +164,13 @@
>> > > #size-cells = <0>;
>> > > };
>> > >
>> > > + i2c@121D0000 {
>> > > + compatible = "samsung,s3c2440-sataphy-i2c";
>> >
>> > Is this a unique i2c controller, or is it just another one like the others
>> > on
>> > the chip? If it's the latter, it should use the regular compatible string.
>> >
>>
>> Yes, its a unique i2c controller which lacks an interrupt line while others
>> are interrupt driven.
>> Hence I have used a distinct compatible string for the driver to
>> distinguish the controller.
>
> It would be better to just make the i2c bus driver handle the case where there
> is no interrupt specifier and just use polling in those cases, especially if
> the rest of the device is identical and doesn't need special handling.
>
> As a matter of fact, if that had been done for the hdmi phy, then you could
> have done this patch without modifying the driver at all, just device tree
> contents. And the same would go for the next time down the road when
> a "special" i2c bus is added.
>
Yes, It is the i2c bus driver thats handles this case. There is a
subsequent patch
that provides the polling support to the driver. As far as I know, the
i2c controller for
HDMI PHY is also interrupt driven.
>
> -Olof
--
Regards,
Vasanth K A
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
