Hi,
Thanks for the replies.
What isn't commented below should already be fixed.
On 13 June 2013 00:49, Olof Johansson <o...@lixom.net> wrote:
> Hi,
>
> You should add a bindings description for the MOXA SoC platforms, similar to
> how others do it, under Documentation/devicetree/bindings/arm/.
The following is now added under Documentation:
Documentation/devicetree/bindings/arm/moxart.txt | 8 +++
.../arm/moxart/moxart-interrupt-controller.txt | 29 ++++++++++++
.../bindings/arm/moxart/moxart-timer.txt | 17 +++++++
> Same goes for other device bindings below -- they all need to be added to the
> bindings documentation. You might be better off removing some of them until
> you
> post and merge the corresponding drivers.
I'll remove them for now with the intent of adding them later. Maybe
when (and if) all drivers are posted and merged.
>> +/dts-v1/;
>> +/include/ "moxart.dtsi"
>> +
>> +/ {
>> + model = "MOXA UC-7112-LX";
>> + compatible = "moxa,moxart-uc-7112-lx";
>
> Is there a generic board design / eval board that you can have as a fallback
> compatible value? That way you won't have to add every board to the table in
> the c file.
There isn't really a generic board but the same can be accomplished by
adding "moxa,moxart" to compatible? New boards can then be added
without patching arch/arm/mach-moxart/moxart.c.
>> + mxser@98200040 {
>> + compatible = "moxa,moxart-mxser";
>> + reg = <0x98200040 0x00000080>, /* UART "3" base */
>> + <0x982000e4 0x00000080>, /* UART mode
>> base */
>> + <0x982000c0 0x00000020>; /* UART
>> interrupt vector */
>
> Are there other registers for other devices inbetween, or could you just do
> one
> large memory area here?
No, reg could simply be 0x98200040 to 0x982000e0. I split them out as
documentation but those could be defined in the driver instead, also
because it's sort of easier to assign values with of_iomap.
>> + interrupts = <31 1>;
>> + };
>> +
>> + mac0: mac@90900000 {
>> + compatible = "moxa,moxart-mac0";
>> + reg = <0x90900000 0x1000>,
>> + <0x80000050 0x5>; /* MAC
>> address stored on flash */
>
> This is a pretty unusal way to indicate mac address location. While I suppose
> it works, I'd like to get the device tree maintainers in the loop. ADding them
> to cc.
>
> Also, there should be a bindings doc for this device (and a driver)
I'm removing ethernet until the driver is submitted. This allows me to
submit bindings doc later?
> I'm also surprised that you have a 5-byte mac address. 6 bytes is much more
> common. :)
That _is_ surprising :) Corrected to 6 bytes.
> Finally, are the two MACs using the same driver? if so, using the same
> compatible value makes more sense.
Yes, using the same driver is the point. I see now they can use the
same value, they'll still be probed from the DT entries.
Best regards,
Jonas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
