Re: [RFC][PATCH] axi: add AXI bus driver

[RafaÅ MiÅecki]

2011/4/12 RafaÅ MiÅecki <zajec5@xxxxxxxxx>:
> 2011/4/12 Hauke Mehrtens <hauke@xxxxxxxxxx>:
>> Hi RafaÅ,
>>
>> On 04/12/2011 09:27 PM, RafaÅ MiÅecki wrote:
>>> 2011/4/12 George Kashperko <george@xxxxxxxxxxx>:
>>>>
>>>>> 2011/4/12 George Kashperko <george@xxxxxxxxxxx>:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Tue, Apr 12, 2011 at 01:57:07AM +0200, RafaÅ MiÅecki wrote:
>>>>>>>> Cc: Michael BÃsch <mb@xxxxxxxxx>
>>>>>>>> Cc: Larry Finger <Larry.Finger@xxxxxxxxxxxx>
>>>>>>>> Cc: George Kashperko <george@xxxxxxxxxxx>
>>>>>>>> Cc: Arend van Spriel <arend@xxxxxxxxxxxx>
>>>>>>>> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>>>>>>>> Cc: Russell King <rmk@xxxxxxxxxxxxxxxx>
>>>>>>>> Cc: Arnd Bergmann <arnd@xxxxxxxx>
>>>>>>>> Cc: Andy Botting <andy@xxxxxxxxxxxxxxx>
>>>>>>>> Cc: linuxdriverproject <devel@xxxxxxxxxxxxxxxxxxxxxx>
>>>>>>>> Cc: linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>
>>>>>>>> Signed-off-by: RafaÅ MiÅecki <zajec5@xxxxxxxxx>
>>>>>>>> ---
>>>>>>>> V2: Rename to axi
>>>>>>>> Â Â Use DEFINE_PCI_DEVICE_TABLE in bridge
>>>>>>>> Â Â Make use of pr_fmt and pr_*
>>>>>>>> Â Â Store core class
>>>>>>>> Â Â Rename bridge to not b43 specific
>>>>>>>> Â Â Replace magic 0x1000 with BCMAI_CORE_SIZE
>>>>>>>> Â Â Remove some old "ssb" names and defines
>>>>>>>> Â Â Move BCMAI_ADDR_BASE def
>>>>>>>> Â Â Add drvdata field
>>>>>>>> V3: Fix reloading (kfree issue)
>>>>>>>> Â Â Add 14e4:0x4331
>>>>>>>> Â Â Fix non-initialized struct issue
>>>>>>>> Â Â Drop useless inline functions wrappers for pci core drv
>>>>>>>> Â Â Proper pr_* usage
>>>>>>>> V3.1: Include forgotten changes (pr_* and include related)
>>>>>>>> Â Â Explain why we dare to implement empty release function
>>>>>>>
>>>>>>> I'm not sure we need this. If you have an IP Core which talks AXI and
>>>>>>> you want to put it on a PCI bus, you will have a PCI Bus wrapper around
>>>>>>> that IP Core, so you should go and let the kernel know about that. See
>>>>>>> [1] for a core IP which talks AXI and [2] for a PCI bus glue layer.
>>>>>>>
>>>>>>> Besides, if you introduce this bus layer, it'll be more difficult for
>>>>>>> other licensees of the same core to re-use the same driver, since it's
>>>>>>> now talking a PCI emulated on top of AXI. The same can be achieved with
>>>>>>> the platform_bus which is more widely used, specially on ARM SoCs.
>>>>>>>
>>>>>>> [1] http://gitorious.org/usb/usb/blobs/dwc3/drivers/usb/dwc3/core.c
>>>>>>> [2] http://gitorious.org/usb/usb/blobs/dwc3/drivers/usb/dwc3/dwc3-haps.c
>>>>>>>
>>>>>>
>>>>>> Already noticed earlier that AXI isnt really good name for
>>>>>> Broadcom-specific axi bus customization. As of tech docs available from
>>>>>> arm, corelink AXI cores use own identification registers which feature
>>>>>> different format and layout comparing to that we use for Broadcom cores.
>>>>>>
>>>>>> Maybe there is something "standartized" by the DMP specs? If so I'm
>>>>>> curious if that DMP is obligatory for every axi bus ?
>>>>>>
>>>>>> Naming particular Broadcom's implementation just axi limits other
>>>>>> licensees in reusing axi bus name/code or will require hacks/workarounds
>>>>>> from them to fit Broadcom-like core scanning/identificating techniques.
>>>>>> You use bus named AXI to group and manage Broadcom cores, while never
>>>>>> even publish device records for native axi cores Broadcom use to talk to
>>>>>> the interconnect through. Yet again, something like bcmb/bcmai looks
>>>>>> like better name for this bus.
>>>>>
>>>>> I don't know, I'm really tired of this. Earlier I was told to not use
>>>>> anything like bcmai, because it is not Broadcom specific. Now it seems
>>>>> (and I'm afraid I agree) there is quite a lot of Broadcom specific
>>>>> stuff.
>>>> Well, _if_ that "magic" EROM core layout is arm's "standard" for axi
>>>> ports identification _and_ _if_ that EROM core is obligatory axi
>>>> component then sure axi name is good one as soon as you consider
>>>> registering master port (agent) cores with device subsystem as well.
>>>> I have no clue here about how resolve those _if_'s, hopefully Broadcom
>>>> guys can enlighten us on the subject.
>>>
>>> Do you think that in my code only scanning is Broadcom specific? In
>>> such a case we could keep it "axi", and just s/scan/bcmscan/. This is
>>> only correct choice if the rest (addressing, core enabling, host
>>> management) is AXI specific.
>>
>> The specification for the AMBA AXI Interface is available for free
>> download from ARM if you register to their website and accept their license:
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.set.amba/index.html
>> I got it from there without any problems and the license does not look
>> too bad for me, by having a quick look at it. I do not know if it will
>> help you in any way or if it is completely unrelated.
>>
>> Why is the existing support for the amba bus not extended or used in any
>> way for this? It exists for some time in drivers/amba/. There already
>> was a discussion about this in https://lkml.org/lkml/2011/3/30/186 , but
>> with no result as I see.
>
> I can see exactly nothing I could use from whatever driver/amba is.
> What does it do from things we need? How do you imagine using that
> with out (non)Broadcom buses?

1) I checked for amba_device_register:
http://lxr.free-electrons.com/ident?i=amba_device_register
and do not understand that. There are a lot of drivers registering
some pre-defined devices. I could not find any driver scanning for
amba devices and registering them. Are we going to be the first driver
registering devices dynamically or do I get this totally wrong

2) amba_id contains only some interesting "id". How can we relate this
with our core id/rev/manuf/class?

3) There is no code for managing AMBA cores (enable, checking status,
disabling, resetting)...

That way I see really low (or not at all) relation between out
(not)Broadcom bus and present AMBA bus.

-- 
RafaÅ
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/