Re: [RESEND v7 14/37] clk: Compatible with narrow registers
m my comments on v6)
> There is no need to increase the size of the flags field for the gate clock.
>
>
>> spinlock_t *lock;
>> };
>>
>
>> @@ -675,13 +681,17 @@ struct clk_div_table {
>> * CLK_DIVIDER_BIG_ENDIAN - By default little endian register accesses are
>> used
>> * for the divider register. Setting this flag makes the register
>> accesses
>> * big endian.
>> + * CLK_DIVIDER_REG_8BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses
>> 8bit.
>> + * CLK_DIVIDER_REG_16BIT - by default 32bit register accesses are used for
>> + * the gate register. Setting this flag makes the register accesses
>> 16bit.
>> */
>> struct clk_divider {
>> struct clk_hw hw;
>> void __iomem*reg;
>> u8 shift;
>> u8 width;
>> - u8 flags;
>> + u16 flags;
>> const struct clk_div_table *table;
>> spinlock_t *lock;
>> };
>
>> @@ -726,18 +738,18 @@ struct clk_hw *__clk_hw_register_divider(struct device
>> *dev,
>> struct device_node *np, const char *name,
>> const char *parent_name, const struct clk_hw *parent_hw,
>> const struct clk_parent_data *parent_data, unsigned long
>> flags,
>> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
>> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
>
> "u16 clk_divider_flags", to match clk_divider.flags.
>
>> const struct clk_div_table *table, spinlock_t *lock);
>> struct clk_hw *__devm_clk_hw_register_divider(struct device *dev,
>> struct device_node *np, const char *name,
>> const char *parent_name, const struct clk_hw *parent_hw,
>> const struct clk_parent_data *parent_data, unsigned long
>> flags,
>> - void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags,
>> + void __iomem *reg, u8 shift, u8 width, u32 clk_divider_flags,
>
> Likewise.
>
>> const struct clk_div_table *table, spinlock_t *lock);
>> struct clk *clk_register_divider_table(struct device *dev, const char *name,
>> const char *parent_name, unsigned long flags,
>> void __iomem *reg, u8 shift, u8 width,
>> - u8 clk_divider_flags, const struct clk_div_table *table,
>> + u32 clk_divider_flags, const struct clk_div_table *table,
>
> Likewise.
>
>> spinlock_t *lock);
>> /**
>> * clk_register_divider - register a divider clock with the clock framework
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
Damien Le Moal
Western Digital Research
Re: [DO NOT MERGE v6 27/37] dt-bindings: ata: ata-generic: Add new targets
On 1/10/24 16:19, Krzysztof Kozlowski wrote: > On 10/01/2024 03:06, Damien Le Moal wrote: >> On 1/9/24 17:23, Yoshinori Sato wrote: >>> Added new ata-generic target. >>> - iodata,usl-5p-ata >>> - renesas,rts7751r2d-ata >>> >>> Each boards have simple IDE Interface. Use ATA generic driver. >> >> This looks OK to me, so feel free to add: >> >> Acked-by: Damien Le Moal >> >> Note: The "DO NOT MERGE" patch prefix almost got me to immediately delete >> this >> 37 patches in my inbox... If you wish to get this work merged after review, >> please use the regular "PATCH" prefix. No worries, the series will not be >> merged >> until is is reviewed :) > > The point of DO NOT MERGE was that feedback was not being implemented > and same set of patches with same issues were kept sending. :/ OK. I found that prefix unusual, but not a big deal. Thanks. > > Best regards, > Krzysztof > -- Damien Le Moal Western Digital Research
Re: [DO NOT MERGE v6 27/37] dt-bindings: ata: ata-generic: Add new targets
On 1/9/24 17:23, Yoshinori Sato wrote: > Added new ata-generic target. > - iodata,usl-5p-ata > - renesas,rts7751r2d-ata > > Each boards have simple IDE Interface. Use ATA generic driver. This looks OK to me, so feel free to add: Acked-by: Damien Le Moal Note: The "DO NOT MERGE" patch prefix almost got me to immediately delete this 37 patches in my inbox... If you wish to get this work merged after review, please use the regular "PATCH" prefix. No worries, the series will not be merged until is is reviewed :) -- Damien Le Moal Western Digital Research
Re: [PATCH v3 28/49] dm zoned: dynamically allocate the dm-zoned-meta shrinker
On 7/28/23 07:59, Dave Chinner wrote: > On Thu, Jul 27, 2023 at 07:20:46PM +0900, Damien Le Moal wrote: >> On 7/27/23 17:55, Qi Zheng wrote: >>>>> goto err; >>>>> } >>>>> + zmd->mblk_shrinker->count_objects = dmz_mblock_shrinker_count; >>>>> + zmd->mblk_shrinker->scan_objects = dmz_mblock_shrinker_scan; >>>>> + zmd->mblk_shrinker->seeks = DEFAULT_SEEKS; >>>>> + zmd->mblk_shrinker->private_data = zmd; >>>>> + >>>>> + shrinker_register(zmd->mblk_shrinker); >>>> >>>> I fail to see how this new shrinker API is better... Why isn't there a >>>> shrinker_alloc_and_register() function ? That would avoid adding all this >>>> code >>>> all over the place as the new API call would be very similar to the current >>>> shrinker_register() call with static allocation. >>> >>> In some registration scenarios, memory needs to be allocated in advance. >>> So we continue to use the previous prealloc/register_prepared() >>> algorithm. The shrinker_alloc_and_register() is just a helper function >>> that combines the two, and this increases the number of APIs that >>> shrinker exposes to the outside, so I choose not to add this helper. >> >> And that results in more code in many places instead of less code + a simple >> inline helper in the shrinker header file... > > It's not just a "simple helper" - it's a function that has to take 6 > or 7 parameters with a return value that must be checked and > handled. > > This was done in the first versions of the patch set - the amount of > code in each caller does not go down and, IMO, was much harder to > read and determine "this is obviously correct" that what we have > now. > >> So not adding that super simple >> helper is not exactly the best choice in my opinion. > > Each to their own - I much prefer the existing style/API over having > to go look up a helper function every time I want to check some > random shrinker has been set up correctly OK. All fair points. -- Damien Le Moal Western Digital Research
Re: [PATCH v3 28/49] dm zoned: dynamically allocate the dm-zoned-meta shrinker
On 7/27/23 17:55, Qi Zheng wrote: >>> goto err; >>> } >>> + zmd->mblk_shrinker->count_objects = dmz_mblock_shrinker_count; >>> + zmd->mblk_shrinker->scan_objects = dmz_mblock_shrinker_scan; >>> + zmd->mblk_shrinker->seeks = DEFAULT_SEEKS; >>> + zmd->mblk_shrinker->private_data = zmd; >>> + >>> + shrinker_register(zmd->mblk_shrinker); >> >> I fail to see how this new shrinker API is better... Why isn't there a >> shrinker_alloc_and_register() function ? That would avoid adding all this >> code >> all over the place as the new API call would be very similar to the current >> shrinker_register() call with static allocation. > > In some registration scenarios, memory needs to be allocated in advance. > So we continue to use the previous prealloc/register_prepared() > algorithm. The shrinker_alloc_and_register() is just a helper function > that combines the two, and this increases the number of APIs that > shrinker exposes to the outside, so I choose not to add this helper. And that results in more code in many places instead of less code + a simple inline helper in the shrinker header file... So not adding that super simple helper is not exactly the best choice in my opinion. -- Damien Le Moal Western Digital Research
Re: [PATCH v3 28/49] dm zoned: dynamically allocate the dm-zoned-meta shrinker
On 7/27/23 17:04, Qi Zheng wrote:
> In preparation for implementing lockless slab shrink, use new APIs to
> dynamically allocate the dm-zoned-meta shrinker, so that it can be freed
> asynchronously using kfree_rcu(). Then it doesn't need to wait for RCU
> read-side critical section when releasing the struct dmz_metadata.
>
> Signed-off-by: Qi Zheng
> Reviewed-by: Muchun Song
> ---
> drivers/md/dm-zoned-metadata.c | 28
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
> index 9d3cca8e3dc9..0bcb26a43578 100644
> --- a/drivers/md/dm-zoned-metadata.c
> +++ b/drivers/md/dm-zoned-metadata.c
> @@ -187,7 +187,7 @@ struct dmz_metadata {
> struct rb_root mblk_rbtree;
> struct list_headmblk_lru_list;
> struct list_headmblk_dirty_list;
> - struct shrinker mblk_shrinker;
> + struct shrinker *mblk_shrinker;
>
> /* Zone allocation management */
> struct mutexmap_lock;
> @@ -615,7 +615,7 @@ static unsigned long dmz_shrink_mblock_cache(struct
> dmz_metadata *zmd,
> static unsigned long dmz_mblock_shrinker_count(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct dmz_metadata *zmd = container_of(shrink, struct dmz_metadata,
> mblk_shrinker);
> + struct dmz_metadata *zmd = shrink->private_data;
>
> return atomic_read(>nr_mblks);
> }
> @@ -626,7 +626,7 @@ static unsigned long dmz_mblock_shrinker_count(struct
> shrinker *shrink,
> static unsigned long dmz_mblock_shrinker_scan(struct shrinker *shrink,
> struct shrink_control *sc)
> {
> - struct dmz_metadata *zmd = container_of(shrink, struct dmz_metadata,
> mblk_shrinker);
> + struct dmz_metadata *zmd = shrink->private_data;
> unsigned long count;
>
> spin_lock(>mblk_lock);
> @@ -2936,19 +2936,23 @@ int dmz_ctr_metadata(struct dmz_dev *dev, int num_dev,
>*/
> zmd->min_nr_mblks = 2 + zmd->nr_map_blocks + zmd->zone_nr_bitmap_blocks
> * 16;
> zmd->max_nr_mblks = zmd->min_nr_mblks + 512;
> - zmd->mblk_shrinker.count_objects = dmz_mblock_shrinker_count;
> - zmd->mblk_shrinker.scan_objects = dmz_mblock_shrinker_scan;
> - zmd->mblk_shrinker.seeks = DEFAULT_SEEKS;
>
> /* Metadata cache shrinker */
> - ret = register_shrinker(>mblk_shrinker, "dm-zoned-meta:(%u:%u)",
> - MAJOR(dev->bdev->bd_dev),
> - MINOR(dev->bdev->bd_dev));
> - if (ret) {
> - dmz_zmd_err(zmd, "Register metadata cache shrinker failed");
> + zmd->mblk_shrinker = shrinker_alloc(0, "dm-zoned-meta:(%u:%u)",
> + MAJOR(dev->bdev->bd_dev),
> + MINOR(dev->bdev->bd_dev));
> + if (!zmd->mblk_shrinker) {
> + dmz_zmd_err(zmd, "Allocate metadata cache shrinker failed");
ret is not set here, so dmz_ctr_metadata() will return success. You need to add:
ret = -ENOMEM;
or something.
> goto err;
> }
>
> + zmd->mblk_shrinker->count_objects = dmz_mblock_shrinker_count;
> + zmd->mblk_shrinker->scan_objects = dmz_mblock_shrinker_scan;
> + zmd->mblk_shrinker->seeks = DEFAULT_SEEKS;
> + zmd->mblk_shrinker->private_data = zmd;
> +
> + shrinker_register(zmd->mblk_shrinker);
I fail to see how this new shrinker API is better... Why isn't there a
shrinker_alloc_and_register() function ? That would avoid adding all this code
all over the place as the new API call would be very similar to the current
shrinker_register() call with static allocation.
> +
> dmz_zmd_info(zmd, "DM-Zoned metadata version %d", zmd->sb_version);
> for (i = 0; i < zmd->nr_devs; i++)
> dmz_print_dev(zmd, i);
> @@ -2995,7 +2999,7 @@ int dmz_ctr_metadata(struct dmz_dev *dev, int num_dev,
> */
> void dmz_dtr_metadata(struct dmz_metadata *zmd)
> {
> - unregister_shrinker(>mblk_shrinker);
> + shrinker_free(zmd->mblk_shrinker);
> dmz_cleanup_metadata(zmd);
> kfree(zmd);
> }
--
Damien Le Moal
Western Digital Research
Re: [PATCH] PCI: Add ASPEED vendor ID
On 4/21/23 04:10, Bjorn Helgaas wrote: > [+cc Damien, linux-ide] > > On Thu, Apr 20, 2023 at 09:08:48AM +0200, Thomas Zimmermann wrote: >> Am 19.04.23 um 20:37 schrieb Bjorn Helgaas: >>> On Wed, Apr 19, 2023 at 09:00:15AM +0200, Thomas Zimmermann wrote: Am 19.04.23 um 00:57 schrieb Patrick McLean: > Currently the ASPEED PCI vendor ID is defined in > drivers/gpu/drm/ast/ast_drv.c, move that to include/linux/pci_ids.h > with all the rest of the PCI vendor ID definitions. Rename the definition > to follow the format that the other definitions follow. Thanks a lot. Can you please also move and rename the PCI device ids? [1] >>> >>> Generally we move things to pci_ids.h only when they are shared >>> between multiple drivers. This is mostly to make backports easier. >>> >>> PCI_VENDOR_ID_ASPEED is (or will be) used in both ast_drv.c and >>> libata-core.c, so it qualifies. >>> >>> It doesn't look like PCI_CHIP_AST2000 and PCI_CHIP_AST2100 would >>> qualify since they're only used in ast_drv.c and ast_main.c, which are >>> part of the same driver. >> >> Ok, I see. Can I take the patch into DRM trees? > > The first time around I got two patches [2]. This time I only got > this patch, but IIUC there are still two patches in play here: > > - This one, which moves PCI_VENDOR_ID_ASPEED to pci_ids.h, and > - The libata-core one that adds a use in ata_dev_config_ncq() > > Those should go together via the same tree. I supplied my ack to > indicate that I'm not going to merge anything myself, and I expect > whoever merges the libata patch to also merge this one. > > If for some reason the libata-core patch doesn't happen, then this > patch shouldn't happen either, because there would no longer be any > sharing between drivers that would justify a pci_ids.h addition. I can take both patches through the libata tree but there were comments on the second patch for libata and I have not seen these addressed yet (I did not get a v2). And in the meantime, it seems that the PCI ID patch was reworked as a single patch... Not sure what's happening here.
Re: [PATCH 2/2] ata: libata-core: Apply ATI NCQ horkage to ASPEED as well
On 4/18/23 14:24, Christoph Hellwig wrote: > On Mon, Apr 17, 2023 at 06:17:20PM -0700, Patrick McLean wrote: >> We have some machines with ASPEED SATA controllers, and are seeing the same >> NCQ >> issues that ATI controllers (I am not sure if it's a rebranded ATI >> controller, >> or they both have some faulty implementation). This NCQ breakage is >> consistent >> across a few different types of drives. >> >> Instead of maintaining a list of drives that are broken with ASPEED >> controllers > > Are these ASPEED controllers all the same or a wide variety? > Quirking all controllers from the same vendor seems like an overly > broad approach to me. Indeed. If you checked only one adapter model from ASPEED, then all that is needed is define it with "board_ahci_noncq" in drivers/ata/ahci.c (see ahci_pci_tbl array). NCQ support will be turned off for that particular adapter with that.
Re: [PATCH 2/5] dt-bindings: ata: drop minItems equal to maxItems
On 8/25/22 20:33, Krzysztof Kozlowski wrote: > minItems, if missing, are implicitly equal to maxItems, so drop > redundant piece to reduce size of code. > > Signed-off-by: Krzysztof Kozlowski Acked-by: Damien Le Moal > --- > Documentation/devicetree/bindings/ata/brcm,sata-brcm.yaml | 1 - > .../devicetree/bindings/ata/cortina,gemini-sata-bridge.yaml | 2 -- > Documentation/devicetree/bindings/ata/sata_highbank.yaml| 1 - > 3 files changed, 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/ata/brcm,sata-brcm.yaml > b/Documentation/devicetree/bindings/ata/brcm,sata-brcm.yaml > index 235a93ac86b0..3766cc80cb17 100644 > --- a/Documentation/devicetree/bindings/ata/brcm,sata-brcm.yaml > +++ b/Documentation/devicetree/bindings/ata/brcm,sata-brcm.yaml > @@ -30,7 +30,6 @@ properties: >- const: brcm,bcm-nsp-ahci > >reg: > -minItems: 2 > maxItems: 2 > >reg-names: > diff --git > a/Documentation/devicetree/bindings/ata/cortina,gemini-sata-bridge.yaml > b/Documentation/devicetree/bindings/ata/cortina,gemini-sata-bridge.yaml > index 21a90975593b..529093666508 100644 > --- a/Documentation/devicetree/bindings/ata/cortina,gemini-sata-bridge.yaml > +++ b/Documentation/devicetree/bindings/ata/cortina,gemini-sata-bridge.yaml > @@ -22,7 +22,6 @@ properties: > maxItems: 1 > >resets: > -minItems: 2 > maxItems: 2 > description: phandles to the reset lines for both SATA bridges > > @@ -32,7 +31,6 @@ properties: >- const: sata1 > >clocks: > -minItems: 2 > maxItems: 2 > description: phandles to the compulsory peripheral clocks > > diff --git a/Documentation/devicetree/bindings/ata/sata_highbank.yaml > b/Documentation/devicetree/bindings/ata/sata_highbank.yaml > index 49679b58041c..f23f26a8f21c 100644 > --- a/Documentation/devicetree/bindings/ata/sata_highbank.yaml > +++ b/Documentation/devicetree/bindings/ata/sata_highbank.yaml > @@ -52,7 +52,6 @@ properties: > minItems: 1 > maxItems: 8 > items: > - minItems: 2 >maxItems: 2 > >calxeda,tx-atten: -- Damien Le Moal Western Digital Research
Re: [PATCH v5 04/13] dt-bindings: memory-controllers: add canaan k210 sram controller
On 7/11/22 04:39, conor.doo...@microchip.com wrote:
> Damien, Krzysztof,
>
> I know this particular version has not been posted for all that
> long, but this binding is (functionally) unchanged for a few
> versions now. Are you happy with this approach Damien?
> U-Boot only cares about the compatible & the clocks property,
> not the regs etc.
>
> I (lazily) tested it in U-Boot with the following diff:
If both the kernel and u-boot still work as expected with this change, I
am OK with it.
>
> diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
> index 3cc8379133..314db88340 100644
> --- a/arch/riscv/dts/k210.dtsi
> +++ b/arch/riscv/dts/k210.dtsi
> @@ -82,11 +82,14 @@
>
> sram: memory@8000 {
> device_type = "memory";
> + reg = <0x8000 0x40>, /* sram0 4 MiB */
> + <0x8040 0x20>, /* sram1 2 MiB */
> + <0x8060 0x20>; /* aisram 2 MiB */
> + u-boot,dm-pre-reloc;
> + };
> +
> + sram_controller: memory-controller {
> compatible = "canaan,k210-sram";
> - reg = <0x8000 0x40>,
> - <0x8040 0x20>,
> - <0x8060 0x20>;
> - reg-names = "sram0", "sram1", "aisram";
> clocks = < K210_CLK_SRAM0>,
> < K210_CLK_SRAM1>,
> < K210_CLK_AI>;
>
> If so, could you queue this for 5.20 please Krzysztof, unless
> you've got concerns about it?
>
> Thanks,
> Conor.
>
> On 05/07/2022 22:52, Conor Dooley wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>>
>> From: Conor Dooley
>>
>> The k210 U-Boot port has been using the clocks defined in the
>> devicetree to bring up the board's SRAM, but this violates the
>> dt-schema. As such, move the clocks to a dedicated node with
>> the same compatible string & document it.
>>
>> Signed-off-by: Conor Dooley
>> ---
>> .../memory-controllers/canaan,k210-sram.yaml | 52 +++
>> 1 file changed, 52 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml
>>
>> diff --git
>> a/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml
>> b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml
>> new file mode 100644
>> index ..f81fb866e319
>> --- /dev/null
>> +++
>> b/Documentation/devicetree/bindings/memory-controllers/canaan,k210-sram.yaml
>> @@ -0,0 +1,52 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/memory-controllers/canaan,k210-sram.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Canaan K210 SRAM memory controller
>> +
>> +description:
>> + The Canaan K210 SRAM memory controller is responsible for the system's 8
>> MiB
>> + of SRAM. The controller is initialised by the bootloader, which configures
>> + its clocks, before OS bringup.
>> +
>> +maintainers:
>> + - Conor Dooley
>> +
>> +properties:
>> + compatible:
>> +enum:
>> + - canaan,k210-sram
>> +
>> + clocks:
>> +minItems: 1
>> +items:
>> + - description: sram0 clock
>> + - description: sram1 clock
>> + - description: aisram clock
>> +
>> + clock-names:
>> +minItems: 1
>> +items:
>> + - const: sram0
>> + - const: sram1
>> + - const: aisram
>> +
>> +required:
>> + - compatible
>> + - clocks
>> + - clock-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> +#include
>> +memory-controller {
>> +compatible = "canaan,k210-sram";
>> +clocks = < K210_CLK_SRAM0>,
>> + < K210_CLK_SRAM1>,
>> + < K210_CLK_AI>;
>> +clock-names = "sram0", "sram1", "aisram";
>> +};
>> --
>> 2.37.0
>>
>
--
Damien Le Moal
Western Digital Research
Re: [PATCH v5 00/13] Canaan devicetree fixes
On 7/6/22 17:03, Geert Uytterhoeven wrote:
> Hi Conor,
>
> On Tue, Jul 5, 2022 at 11:52 PM Conor Dooley wrote:
>> I *DO NOT* have any Canaan hardware so I have not tested any of this in
>> action. Since I sent v1, I tried to buy some since it's cheap - but could
>> out of the limited stockists none seemed to want to deliver to Ireland :(
>> I based the series on next-20220617.
>
> Digi-Key does not want to ship to IRL?
> The plain MAiX BiT is out-of-stock, but the kit incl. a display is
> available (97 in stock).
Seedstudio is out of stock on the MAIX bit, but they have maixduino, which
is the same, almost (pin wiring differs, everything else is the same).
https://www.seeedstudio.com/Sipeed-Maixduino-Kit-for-RISC-V-AI-IoT-p-4047.html
And you can still find plenty of MAIX bit on Aliexpress too.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
>
> ___
> linux-riscv mailing list
> linux-ri...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Damien Le Moal
Western Digital Research
Re: [PATCH v3 00/15] Canaan devicetree fixes
On 7/1/22 02:53, Sudeep Holla wrote:
> On Thu, Jun 30, 2022 at 04:28:26PM +, Niklas Cassel wrote:
>> On Wed, Jun 29, 2022 at 07:43:29PM +0100, Conor Dooley wrote:
>>> From: Conor Dooley
>>>
>>> Hey all,
>>> This series should rid us of dtbs_check errors for the RISC-V Canaan k210
>>> based boards. To make keeping it that way a little easier, I changed the
>>> Canaan devicetree Makefile so that it would build all of the devicetrees
>>> in the directory if SOC_CANAAN.
>>>
>>> I *DO NOT* have any Canaan hardware so I have not tested any of this in
>>> action. Since I sent v1, I tried to buy some since it's cheap - but could
>>> out of the limited stockists none seemed to want to deliver to Ireland :(
>>> I based the series on next-20220617.
>>>
>>
>> I first tried to apply your series on top of next-20220630,
>> but was greeted by a bunch of different warnings on boot,
>> including endless RCU stall warnings.
>> However, even when booting next-20220630 without your patches,
>> I got the same warnings and RCU stall.
>>
>
> Is it possible to share the boot logs please ?
> Conor is having issues with my arch_topology/cacheinfo updates in -next.
> I would like to know if your issue is related to that or not ?
FYI, I see rcu warnings on boot on my dual-socket 8-cores Xeon system, but
the same kernel does not have the rcu warnings with an AMD Epyc single
socket 16-cores box.
>
>> So I tested your series on top of v5.19-rc4 +
>> commit 0397d50f4cad ("spi: dt-bindings: Move 'rx-sample-delay-ns' to
>> spi-peripheral-props.yaml") cherry-picked,
>> (in order to avoid conflicts when applying your series,)
>> and the board was working as intended, no warnings or RCU stalls.
>>
>
> If possible can you give this branch[1] a try where my changes are and doesn't
> have any other changes from -next. Sorry to bother you.
>
> Conor seem to have issue with this commit[2], so if you get issues try to
> check if [3] works.
>
> Regards,
> Sudeep
>
> [1] https://git.kernel.org/sudeep.holla/c/ae85abf284e7
> [2] https://git.kernel.org/sudeep.holla/c/155bd845d17b
> [3] https://git.kernel.org/sudeep.holla/c/009297d29faa
--
Damien Le Moal
Western Digital Research
Re: [PATCH 06/14] spi: dt-bindings: dw-apb-ssi: update spi-{r,t}x-bus-width for dwc-ssi
On 6/21/22 07:49, Conor Dooley wrote:
>
>
> On 20/06/2022 23:46, Damien Le Moal wrote:
>> On 6/21/22 06:06, conor.doo...@microchip.com wrote:
>>> On 20/06/2022 21:56, Serge Semin wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>>>> content is safe
>>>>
>>>> On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
>>>>> From: Conor Dooley
>>>>>
>>>>> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
>>>>> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
>>>>> this.
>>>>>
>>>>> Signed-off-by: Conor Dooley
>>>>> ---
>>>>> .../bindings/spi/snps,dw-apb-ssi.yaml | 48 ++-
>>>>> 1 file changed, 35 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>>> b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>>> index e25d44c218f2..f2b9e3f062cd 100644
>>>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>>> @@ -135,19 +135,41 @@ properties:
>>>>>of the designware controller, and the upper limit is also subject
>>>>> to
>>>>>controller configuration.
>>>>>
>>>>> -patternProperties:
>>>>> - "^.*@[0-9a-f]+$":
>>>>> -type: object
>>>>> -properties:
>>>>> - reg:
>>>>> -minimum: 0
>>>>> -maximum: 3
>>>>> -
>>>>> - spi-rx-bus-width:
>>>>> -const: 1
>>>>> -
>>>>> - spi-tx-bus-width:
>>>>> -const: 1
>>>>> +if:
>>>>> + properties:
>>>>> +compatible:
>>>>> + contains:
>>>>> +const: snps,dwc-ssi-1.01a
>>>>> +
>>>>> +then:
>>>>> + patternProperties:
>>>>> +"^.*@[0-9a-f]+$":
>>>>> + type: object
>>>>> + properties:
>>>>> +reg:
>>>>> + minimum: 0
>>>>> + maximum: 3
>>>>> +
>>>>> +spi-rx-bus-width:
>>>>> + const: 4
>>>>> +
>>>>> +spi-tx-bus-width:
>>>>> + const: 4
>>>>> +
>>>>> +else:
>>>>> + patternProperties:
>>>>> +"^.*@[0-9a-f]+$":
>>>>> + type: object
>>>>> + properties:
>>>>> +reg:
>>>>> + minimum: 0
>>>>> + maximum: 3
>>>>> +
>>>>> +spi-rx-bus-width:
>>>>> + const: 1
>>>>> +
>>>>> +spi-tx-bus-width:
>>>>> + const: 1
>>>>
>>>> You can just use a more relaxed constraint "enum: [1 2 4 8]" here
>>>
>>> 8 too? sure.
>>>
>>>> irrespective from the compatible string. The modern DW APB SSI
>>>> controllers of v.4.* and newer also support the enhanced SPI Modes too
>>>> (Dual, Quad and Octal). Since the IP-core version is auto-detected at
>>>> run-time there is no way to create a DT-schema correctly constraining
>>>> the Rx/Tx SPI bus widths. So let's keep the
>>>> compatible-string-independent "patternProperties" here but just extend
>>>> the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
>>>> properties values.
>>>
>>> SGTM!
>>>
>>>>
>>>> Note the DW APB SSI/AHB SSI driver currently doesn't support the
>>>> enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
>>>> bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
>>>> DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
>>>> activated by means of the corresponding CSR. So most likely the DW AHB
>>>> SSI controllers need some specific setups too.
>>>
>>> hmm, well I'll leave that up to people that have Canaan hardware!
>>
>> I will test this series.
>>
>
> Cool, thanks.
> I'll try to get a respin out tomorrow w/ the memory node "unfixed".
OK. I will test that then :)
> Conor.
>
>>> Thanks,
>>> Conor.
>>>
>>>>
>>>> -Sergey
>>>>
>>>>>
>>>>> unevaluatedProperties: false
>>>>>
>>>>> --
>>>>> 2.36.1
>>>>>
>>>
>>
>>
--
Damien Le Moal
Western Digital Research
Re: [PATCH 06/14] spi: dt-bindings: dw-apb-ssi: update spi-{r,t}x-bus-width for dwc-ssi
On 6/21/22 06:06, conor.doo...@microchip.com wrote:
> On 20/06/2022 21:56, Serge Semin wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>>
>> On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
>>> From: Conor Dooley
>>>
>>> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
>>> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
>>> this.
>>>
>>> Signed-off-by: Conor Dooley
>>> ---
>>> .../bindings/spi/snps,dw-apb-ssi.yaml | 48 ++-
>>> 1 file changed, 35 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> index e25d44c218f2..f2b9e3f062cd 100644
>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> @@ -135,19 +135,41 @@ properties:
>>>of the designware controller, and the upper limit is also subject to
>>>controller configuration.
>>>
>>> -patternProperties:
>>> - "^.*@[0-9a-f]+$":
>>> -type: object
>>> -properties:
>>> - reg:
>>> -minimum: 0
>>> -maximum: 3
>>> -
>>> - spi-rx-bus-width:
>>> -const: 1
>>> -
>>> - spi-tx-bus-width:
>>> -const: 1
>>> +if:
>>> + properties:
>>> +compatible:
>>> + contains:
>>> +const: snps,dwc-ssi-1.01a
>>> +
>>> +then:
>>> + patternProperties:
>>> +"^.*@[0-9a-f]+$":
>>> + type: object
>>> + properties:
>>> +reg:
>>> + minimum: 0
>>> + maximum: 3
>>> +
>>> +spi-rx-bus-width:
>>> + const: 4
>>> +
>>> +spi-tx-bus-width:
>>> + const: 4
>>> +
>>> +else:
>>> + patternProperties:
>>> +"^.*@[0-9a-f]+$":
>>> + type: object
>>> + properties:
>>> +reg:
>>> + minimum: 0
>>> + maximum: 3
>>> +
>>> +spi-rx-bus-width:
>>> + const: 1
>>> +
>>> +spi-tx-bus-width:
>>> + const: 1
>>
>> You can just use a more relaxed constraint "enum: [1 2 4 8]" here
>
> 8 too? sure.
>
>> irrespective from the compatible string. The modern DW APB SSI
>> controllers of v.4.* and newer also support the enhanced SPI Modes too
>> (Dual, Quad and Octal). Since the IP-core version is auto-detected at
>> run-time there is no way to create a DT-schema correctly constraining
>> the Rx/Tx SPI bus widths. So let's keep the
>> compatible-string-independent "patternProperties" here but just extend
>> the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
>> properties values.
>
> SGTM!
>
>>
>> Note the DW APB SSI/AHB SSI driver currently doesn't support the
>> enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
>> bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
>> DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
>> activated by means of the corresponding CSR. So most likely the DW AHB
>> SSI controllers need some specific setups too.
>
> hmm, well I'll leave that up to people that have Canaan hardware!
I will test this series.
> Thanks,
> Conor.
>
>>
>> -Sergey
>>
>>>
>>> unevaluatedProperties: false
>>>
>>> --
>>> 2.36.1
>>>
>
--
Damien Le Moal
Western Digital Research
Re: [PATCH 07/14] riscv: dts: canaan: fix the k210's memory node
On 6/20/22 08:54, conor.doo...@microchip.com wrote:
> On 20/06/2022 00:38, Damien Le Moal wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>>
>> On 6/18/22 21:30, Conor Dooley wrote:
>>> From: Conor Dooley
>>>
>>> The k210 memory node has a compatible string that does not match with
>>> any driver or dt-binding & has several non standard properties.
>>> Replace the reg names with a comment and delete the rest.
>>>
>>> Signed-off-by: Conor Dooley
>>> ---
>>> ---
>>> arch/riscv/boot/dts/canaan/k210.dtsi | 6 --
>>> 1 file changed, 6 deletions(-)
>>>
>>> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi
>>> b/arch/riscv/boot/dts/canaan/k210.dtsi
>>> index 44d338514761..287ea6eebe47 100644
>>> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
>>> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
>>> @@ -69,15 +69,9 @@ cpu1_intc: interrupt-controller {
>>>
>>> sram: memory@8000 {
>>> device_type = "memory";
>>> - compatible = "canaan,k210-sram";
>>> reg = <0x8000 0x40>,
>>> <0x8040 0x20>,
>>> <0x8060 0x20>;
>>> - reg-names = "sram0", "sram1", "aisram";
>>> - clocks = < K210_CLK_SRAM0>,
>>> - < K210_CLK_SRAM1>,
>>> - < K210_CLK_AI>;
>>> - clock-names = "sram0", "sram1", "aisram";
>>> };
>>
>> These are used by u-boot to setup the memory clocks and initialize the
>> aisram. Sure the kernel actually does not use this, but to be in sync with
>> u-boot DT, I would prefer keeping this as is. Right now, u-boot *and* the
>> kernel work fine with both u-boot internal DT and the kernel DT.
>
> Right, but unfortunately that desire alone doesn't do anything about
> the dtbs_check complaints.
>
> I guess the alternative approach of actually documenting the compatible
> would be more palatable?
Yes, I think so. That would allow keeping the fields without the DTB build
warnings.
>
> Thanks,
> Conor.
>
>
>
--
Damien Le Moal
Western Digital Research
Re: [PATCH 07/14] riscv: dts: canaan: fix the k210's memory node
On 6/18/22 21:30, Conor Dooley wrote:
> From: Conor Dooley
>
> The k210 memory node has a compatible string that does not match with
> any driver or dt-binding & has several non standard properties.
> Replace the reg names with a comment and delete the rest.
>
> Signed-off-by: Conor Dooley
> ---
> ---
> arch/riscv/boot/dts/canaan/k210.dtsi | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/arch/riscv/boot/dts/canaan/k210.dtsi
> b/arch/riscv/boot/dts/canaan/k210.dtsi
> index 44d338514761..287ea6eebe47 100644
> --- a/arch/riscv/boot/dts/canaan/k210.dtsi
> +++ b/arch/riscv/boot/dts/canaan/k210.dtsi
> @@ -69,15 +69,9 @@ cpu1_intc: interrupt-controller {
>
> sram: memory@8000 {
> device_type = "memory";
> - compatible = "canaan,k210-sram";
> reg = <0x8000 0x40>,
> <0x8040 0x20>,
> <0x8060 0x20>;
> - reg-names = "sram0", "sram1", "aisram";
> - clocks = < K210_CLK_SRAM0>,
> - < K210_CLK_SRAM1>,
> - < K210_CLK_AI>;
> - clock-names = "sram0", "sram1", "aisram";
> };
These are used by u-boot to setup the memory clocks and initialize the
aisram. Sure the kernel actually does not use this, but to be in sync with
u-boot DT, I would prefer keeping this as is. Right now, u-boot *and* the
kernel work fine with both u-boot internal DT and the kernel DT.
--
Damien Le Moal
Western Digital Research
Re: [PATCH 13/48] ARM: pxa: use pdev resource for palmld mmio
On 4/20/22 01:37, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The palmld header is almost unused in drivers, the only
> remaining thing now is the PATA device address, which should
> really be passed as a resource.
>
> Cc: Jens Axboe
> Cc: linux-...@vger.kernel.org
> Acked-by: Robert Jarzmik
> Acked-by: Bartlomiej Zolnierkiewicz
> Signed-off-by: Arnd Bergmann
> ---
> arch/arm/mach-pxa/palmld-pcmcia.c | 3 ++-
> arch/arm/mach-pxa/palmld.c| 12 +---
> arch/arm/mach-pxa/{include/mach => }/palmld.h | 2 +-
> drivers/ata/pata_palmld.c | 3 +--
> 4 files changed, 13 insertions(+), 7 deletions(-)
> rename arch/arm/mach-pxa/{include/mach => }/palmld.h (98%)
>
> diff --git a/arch/arm/mach-pxa/palmld-pcmcia.c
> b/arch/arm/mach-pxa/palmld-pcmcia.c
> index 07e0f7438db1..720294a50864 100644
> --- a/arch/arm/mach-pxa/palmld-pcmcia.c
> +++ b/arch/arm/mach-pxa/palmld-pcmcia.c
> @@ -13,9 +13,10 @@
> #include
>
> #include
> -#include
> #include
>
> +#include "palmld.h"
> +
> static struct gpio palmld_pcmcia_gpios[] = {
> { GPIO_NR_PALMLD_PCMCIA_POWER, GPIOF_INIT_LOW, "PCMCIA Power" },
> { GPIO_NR_PALMLD_PCMCIA_RESET, GPIOF_INIT_HIGH,"PCMCIA Reset" },
> diff --git a/arch/arm/mach-pxa/palmld.c b/arch/arm/mach-pxa/palmld.c
> index d85146957004..d821606ce0b5 100644
> --- a/arch/arm/mach-pxa/palmld.c
> +++ b/arch/arm/mach-pxa/palmld.c
> @@ -29,8 +29,8 @@
> #include
>
> #include "pxa27x.h"
> +#include "palmld.h"
> #include
> -#include
> #include
> #include
> #include
> @@ -279,9 +279,15 @@ static inline void palmld_leds_init(void) {}
> * HDD
>
> **/
> #if defined(CONFIG_PATA_PALMLD) || defined(CONFIG_PATA_PALMLD_MODULE)
> +static struct resource palmld_ide_resources[] = {
> + DEFINE_RES_MEM(PALMLD_IDE_PHYS, 0x1000),
> +};
> +
> static struct platform_device palmld_ide_device = {
> - .name = "pata_palmld",
> - .id = -1,
> + .name = "pata_palmld",
> + .id = -1,
> + .resource = palmld_ide_resources,
> + .num_resources = ARRAY_SIZE(palmld_ide_resources),
> };
>
> static struct gpiod_lookup_table palmld_ide_gpio_table = {
> diff --git a/arch/arm/mach-pxa/include/mach/palmld.h
> b/arch/arm/mach-pxa/palmld.h
> similarity index 98%
> rename from arch/arm/mach-pxa/include/mach/palmld.h
> rename to arch/arm/mach-pxa/palmld.h
> index 99a6d8b3a1e3..ee3bc15b71a2 100644
> --- a/arch/arm/mach-pxa/include/mach/palmld.h
> +++ b/arch/arm/mach-pxa/palmld.h
> @@ -9,7 +9,7 @@
> #ifndef _INCLUDE_PALMLD_H_
> #define _INCLUDE_PALMLD_H_
>
> -#include "irqs.h" /* PXA_GPIO_TO_IRQ */
> +#include /* PXA_GPIO_TO_IRQ */
>
> /** HERE ARE GPIOs **/
>
> diff --git a/drivers/ata/pata_palmld.c b/drivers/ata/pata_palmld.c
> index 2448441571ed..400e65190904 100644
> --- a/drivers/ata/pata_palmld.c
> +++ b/drivers/ata/pata_palmld.c
> @@ -25,7 +25,6 @@
> #include
>
> #include
> -#include
>
> #define DRV_NAME "pata_palmld"
>
> @@ -63,7 +62,7 @@ static int palmld_pata_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> /* remap drive's physical memory address */
> - mem = devm_ioremap(dev, PALMLD_IDE_PHYS, 0x1000);
> + mem = devm_platform_ioremap_resource(pdev, 0);
> if (!mem)
> return -ENOMEM;
>
Acked-by: Damien Le Moal
--
Damien Le Moal
Western Digital Research
Re: [PATCH RFC PKS/PMEM 26/58] fs/zonefs: Utilize new kmap_thread()
On 2020/10/10 4:52, ira.we...@intel.com wrote: > From: Ira Weiny > > The kmap() calls in this FS are localized to a single thread. To avoid > the over head of global PKRS updates use the new kmap_thread() call. > > Cc: Damien Le Moal > Cc: Naohiro Aota > Signed-off-by: Ira Weiny > --- > fs/zonefs/super.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c > index 8ec7c8f109d7..2fd6c86beee1 100644 > --- a/fs/zonefs/super.c > +++ b/fs/zonefs/super.c > @@ -1297,7 +1297,7 @@ static int zonefs_read_super(struct super_block *sb) > if (ret) > goto free_page; > > - super = kmap(page); > + super = kmap_thread(page); > > ret = -EINVAL; > if (le32_to_cpu(super->s_magic) != ZONEFS_MAGIC) > @@ -1349,7 +1349,7 @@ static int zonefs_read_super(struct super_block *sb) > ret = 0; > > unmap: > - kunmap(page); > + kunmap_thread(page); > free_page: > __free_page(page); > > acked-by: Damien Le Moal -- Damien Le Moal Western Digital Research ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
