Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 19 Dec 2013 12:23:48 +0100
From:      Zbigniew Bodek <zbb@semihalf.com>
To:        Warner Losh <imp@bsdimp.com>
Cc:        "freebsd-arm@freebsd.org" <freebsd-arm@freebsd.org>, br@freebsd.org, Olivier Houchard <cognet@FreeBSD.org>
Subject:   Re: RFC: ARM related fixes - GIC, cache line size, PCI FDT & AHCI
Message-ID:  <52B2D744.3090003@semihalf.com>
In-Reply-To: <F9108B73-5293-4118-8281-6140A7658C1A@bsdimp.com>
References:  <52AF3D06.2000004@semihalf.com> <F9108B73-5293-4118-8281-6140A7658C1A@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help
Hello Warner,

Thanks for your review.
Please see below.

Best regards
zbb

On 16.12.2013 22:50, Warner Losh wrote:
> 
> On Dec 16, 2013, at 10:48 AM, Zbigniew Bodek wrote:
> 
>> Hello Everyone.
>>
>> We would like to submit some new patches recently developed by Semihalf.
> 
> Thanks for your efforts here. Would there be a need to MFC these patches to 10 and/or 9?
[ZBB] We can take a look at that after committing to current.
> 
>> You can find them here:
>> http://people.freebsd.org/~zbb/Semihalf/12.2013/
>>
>> Detailed description is available in the commit logs but in general:
>>
>> -- 0001-Resolve-cache-line-size-using-CP15.patch
>>   - use cache line size acquired in runtime
> 
> This looks good (assuming the register definitions are good, I didn't verify them). Might be useful to publish these values to userland somehow, maybe via a sysctl. This is option, but would be nice to have.
> 
> Would also be good to have USB_ALIGN kassert if using USB and this value is < that macro.
> 
> Do other, older CPUs need similar decoding, or are the hard-wired values good?
[ZBB] I have no knowledge about that. Someone who is using older ARMs
should take a look.
> 
>> -- 0002-GIC-polarity-and-level-support.patch
>>   - suport for setting trigger level and polarity in GIC
> 
> This one seems to be OK, but is complicated enough that more eyeballs and testing would be good....
[ZBB] OK. Someone is testing this right now.
> 
>> -- 0003-Add-PCI-FDT-interrupt-trigger-polarity-parsing.patch
>>   - trigger and polarity parsing for PCI FDT interrupts
> 
> This patch might conflict.
> 
> +		if (trig)
> +			*trig = tmp_trig;
> +		if (pol)
> +			*pol = tmp_pol;
> 
> Are not zero values also valid?
[ZBB] Yeah. We will skip this patch for now.
> 
>> -- 0004-Do-not-attach-to-bridges-in-AHCI-driver.patch
> 
> I'd add the word 'PCI' before bridge in the comment, otherwise this looks good.
[ZBB] OK.
> 
>> -- 0005-Use-only-mapped-BIOs-on-ARM.patch
> 
> A comment about what MVA is would be useful.. Otherwise does what it says.
[ZBB]. This should refer to just Virtual Address (not "Modified") so we
will change it back to VA.
> 
>>  - Two patches enabling the AHCI driver on ARM chips
> 
> I didn't see these. Did I miss something? Or is the unmapped stuff all it takes?
[ZBB] Yes, I meant the two above(0004 && 0005).
> 
>> We will appreciate if you could post your comments and/or remarks by the
>> end of this week when we plan to commit the changes.
>>
>> Best regards
>> zbb
>> _______________________________________________
>> freebsd-arm@freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-arm
>> To unsubscribe, send any mail to "freebsd-arm-unsubscribe@freebsd.org"
> 




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?52B2D744.3090003>