| Message ID | EE11001F9E5DDD47B7634E2F8A612F2E1F881744@lhreml507-mbx |
|---|---|
| State | New |
| Headers | show |
On Thursday, September 22, 2016 11:55:45 AM CEST Gabriele Paoloni wrote: > > > I think extending of_empty_ranges_quirk() may be a reasonable > > solution. > > > What do you think Arnd? > > > > I don't really like that idea, that quirk is meant to work around > > broken DTs, but we can just make the DT valid and implement the > > code properly. > > Ok I understand your point where it is not right to use of_empty_ranges_quirk() > As a quirk is used to work around broken HW or broken FW (as in this case) > rather than to fix code > > What about the following? I think adding the check you suggested next to > of_empty_ranges_quirk() is adding the case we need in the right point (thus > avoiding any duplication) > > --- a/drivers/of/address.c > +++ b/drivers/of/address.c > @@ -457,6 +457,15 @@ static struct of_bus *of_match_bus(struct device_node *np) > return NULL; > } > > +static inline int of_isa_indirect_io(struct device_node *np) > +{ > + /* > + * check if the current node is an isa bus and if indirectio operation > + * are registered > + */ > + return (of_bus_isa_match(np) && arm64_extio_ops); > +} > + > static int of_empty_ranges_quirk(struct device_node *np) > { > if (IS_ENABLED(CONFIG_PPC)) { > @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus, > * This code is only enabled on powerpc. --gcl > */ > ranges = of_get_property(parent, rprop, &rlen); > - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { > + if (ranges == NULL && !of_empty_ranges_quirk(parent) && !of_isa_indirect_io(parent)) { > pr_debug("OF: no ranges; cannot translate\n"); > return 1; > } I don't see what effect that would have. What do you want to achieve with this? I think all we need from this function is to return '1' if we hit an ISA I/O window, and that should happen for the two interesting cases, either no 'ranges' at all, or no translation for the range in question, so that __of_translate_address can return OF_BAD_ADDR, and we can enter the special case handling in the caller, that handles it like Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.htmldiff --git a/drivers/of/address.c b/drivers/of/address.c index 02b2903fe9d2..a18d96843fae 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -685,17 +685,24 @@ static int __of_address_to_resource(struct device_node *dev, if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0) return -EINVAL; taddr = of_translate_address(dev, addrp); - if (taddr == OF_BAD_ADDR) - return -EINVAL; memset(r, 0, sizeof(struct resource)); + if (flags & IORESOURCE_IO) { unsigned long port; - port = pci_address_to_pio(taddr); + + if (taddr == OF_BAD_ADDR) + port = arch_of_address_to_pio(dev, addrp) + else + port = pci_address_to_pio(taddr); + if (port == (unsigned long)-1) return -EINVAL; r->start = port; r->end = port + size - 1; } else { + if (taddr == OF_BAD_ADDR) + return -EINVAL; + r->start = taddr; r->end = taddr + size - 1; }
Hi Arnd > -----Original Message----- > From: Arnd Bergmann [mailto:arnd@arndb.de] > Sent: 22 September 2016 13:15 > To: Gabriele Paoloni > Cc: zhichang; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; lorenzo.pieralisi@arm.com; minyard@acm.org; > linux-pci@vger.kernel.org; gregkh@linuxfoundation.org; John Garry; > will.deacon@arm.com; linux-kernel@vger.kernel.org; Yuanzhichang; > Linuxarm; xuwei (O); linux-serial@vger.kernel.org; > benh@kernel.crashing.org; zourongrong@gmail.com; liviu.dudau@arm.com; > kantyzc@163.com > Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on > Hip06 > > On Thursday, September 22, 2016 11:55:45 AM CEST Gabriele Paoloni > wrote: > > > > I think extending of_empty_ranges_quirk() may be a reasonable > > > solution. > > > > What do you think Arnd? > > > > > > I don't really like that idea, that quirk is meant to work around > > > broken DTs, but we can just make the DT valid and implement the > > > code properly. > > > > Ok I understand your point where it is not right to use > of_empty_ranges_quirk() > > As a quirk is used to work around broken HW or broken FW (as in this > case) > > rather than to fix code > > > > What about the following? I think adding the check you suggested next > to > > of_empty_ranges_quirk() is adding the case we need in the right point > (thus > > avoiding any duplication) > > > > --- a/drivers/of/address.c > > +++ b/drivers/of/address.c > > @@ -457,6 +457,15 @@ static struct of_bus *of_match_bus(struct > device_node *np) > > return NULL; > > } > > > > +static inline int of_isa_indirect_io(struct device_node *np) > > +{ > > + /* > > + * check if the current node is an isa bus and if indirectio > operation > > + * are registered > > + */ > > + return (of_bus_isa_match(np) && arm64_extio_ops); > > +} > > + > > static int of_empty_ranges_quirk(struct device_node *np) > > { > > if (IS_ENABLED(CONFIG_PPC)) { > > @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node > *parent, struct of_bus *bus, > > * This code is only enabled on powerpc. --gcl > > */ > > ranges = of_get_property(parent, rprop, &rlen); > > - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { > > + if (ranges == NULL && !of_empty_ranges_quirk(parent) && > !of_isa_indirect_io(parent)) { > > pr_debug("OF: no ranges; cannot translate\n"); > > return 1; > > } > > I don't see what effect that would have. What do you want to > achieve with this? If I read the code correctly adding the function above would end up in a 1:1 mapping: http://lxr.free-electrons.com/source/drivers/of/address.c#L513 so taddr will be assigned with the cpu address space specified in the children nodes of LPC and we are not using a quirk function (we are just checking that we have the indirect io assigned and that we are on a ISA bus). Now probably there is a nit in my code sketch where of_isa_indirect_io should be probably an architecture specific function... > > I think all we need from this function is to return '1' if > we hit an ISA I/O window, and that should happen for the two > interesting cases, either no 'ranges' at all, or no translation > for the range in question, so that __of_translate_address can > return OF_BAD_ADDR, and we can enter the special case > handling in the caller, that handles it like > I don't think this is very right as you may fail for different reasons other than a missing range property, e.g: http://lxr.free-electrons.com/source/drivers/of/address.c#L575 And even if the only failure case was a missing range if in the future __of_translate_address had to be reworked we would again make a wrong assumption...you get my point? Thanks Gab > diff --git a/drivers/of/address.c b/drivers/of/address.c > index 02b2903fe9d2..a18d96843fae 100644 > --- a/drivers/of/address.c > +++ b/drivers/of/address.c > @@ -685,17 +685,24 @@ static int __of_address_to_resource(struct > device_node *dev, > if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0) > return -EINVAL; > taddr = of_translate_address(dev, addrp); > - if (taddr == OF_BAD_ADDR) > - return -EINVAL; > memset(r, 0, sizeof(struct resource)); > + > if (flags & IORESOURCE_IO) { > unsigned long port; > - port = pci_address_to_pio(taddr); > + > + if (taddr == OF_BAD_ADDR) > + port = arch_of_address_to_pio(dev, addrp) > + else > + port = pci_address_to_pio(taddr); > + > if (port == (unsigned long)-1) > return -EINVAL; > r->start = port; > r->end = port + size - 1; > } else { > + if (taddr == OF_BAD_ADDR) > + return -EINVAL; > + > r->start = taddr; > r->end = taddr + size - 1; > } > > > > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, September 22, 2016 2:47:14 PM CEST Gabriele Paoloni wrote: > > > static int of_empty_ranges_quirk(struct device_node *np) > > > { > > > if (IS_ENABLED(CONFIG_PPC)) { > > > @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node > > *parent, struct of_bus *bus, > > > * This code is only enabled on powerpc. --gcl > > > */ > > > ranges = of_get_property(parent, rprop, &rlen); > > > - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { > > > + if (ranges == NULL && !of_empty_ranges_quirk(parent) && > > !of_isa_indirect_io(parent)) { > > > pr_debug("OF: no ranges; cannot translate\n"); > > > return 1; > > > } > > > > I don't see what effect that would have. What do you want to > > achieve with this? > > If I read the code correctly adding the function above would end > up in a 1:1 mapping: > http://lxr.free-electrons.com/source/drivers/of/address.c#L513 > > so taddr will be assigned with the cpu address space specified > in the children nodes of LPC and we are not using a quirk function > (we are just checking that we have the indirect io assigned and > that we are on a ISA bus). Now probably there is a nit in my > code sketch where of_isa_indirect_io should be probably an architecture > specific function... But the point is that it would then return an incorrect address, which in the worst case could be the same as another I/O space if that happens to be at CPU address zero. > > I think all we need from this function is to return '1' if > > we hit an ISA I/O window, and that should happen for the two > > interesting cases, either no 'ranges' at all, or no translation > > for the range in question, so that __of_translate_address can > > return OF_BAD_ADDR, and we can enter the special case > > handling in the caller, that handles it like > > > > I don't think this is very right as you may fail for different > reasons other than a missing range property, e.g: > http://lxr.free-electrons.com/source/drivers/of/address.c#L575 > > And even if the only failure case was a missing range if in the > future __of_translate_address had to be reworked we would again > make a wrong assumption...you get my point? The newly introduced function would clearly have to make some sanity checks. The idea is that treat the case of not being able to translate a bus specific I/O address into a CPU address literally and fall back to another method of translating that address. This matches my mental model of how we find the resource: - start with the bus address - try to translate that into a CPU address - if we arrive at a CPU physical address for IORESOURCE_MEM, use that - if we arrive at a CPU physical address for IORESOURCE_IO, translate that into a Linux IORESOURCE_IO token - if there is no valid CPU physical address, try to translate the address into an IORESOURCE_IO using the ISA accessor - if that fails too, give up. If you try to fake a CPU physical address inbetween, it just gets more confusing. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Arnd Bergmann [mailto:arnd@arndb.de] > Sent: 22 September 2016 15:59 > To: Gabriele Paoloni > Cc: zhichang; linux-arm-kernel@lists.infradead.org; > devicetree@vger.kernel.org; lorenzo.pieralisi@arm.com; minyard@acm.org; > linux-pci@vger.kernel.org; gregkh@linuxfoundation.org; John Garry; > will.deacon@arm.com; linux-kernel@vger.kernel.org; Yuanzhichang; > Linuxarm; xuwei (O); linux-serial@vger.kernel.org; > benh@kernel.crashing.org; zourongrong@gmail.com; liviu.dudau@arm.com; > kantyzc@163.com > Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on > Hip06 > > On Thursday, September 22, 2016 2:47:14 PM CEST Gabriele Paoloni wrote: > > > > static int of_empty_ranges_quirk(struct device_node *np) > > > > { > > > > if (IS_ENABLED(CONFIG_PPC)) { > > > > @@ -503,7 +512,7 @@ static int of_translate_one(struct > device_node > > > *parent, struct of_bus *bus, > > > > * This code is only enabled on powerpc. --gcl > > > > */ > > > > ranges = of_get_property(parent, rprop, &rlen); > > > > - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { > > > > + if (ranges == NULL && !of_empty_ranges_quirk(parent) && > > > !of_isa_indirect_io(parent)) { > > > > pr_debug("OF: no ranges; cannot translate\n"); > > > > return 1; > > > > } > > > > > > I don't see what effect that would have. What do you want to > > > achieve with this? > > > > If I read the code correctly adding the function above would end > > up in a 1:1 mapping: > > http://lxr.free-electrons.com/source/drivers/of/address.c#L513 > > > > so taddr will be assigned with the cpu address space specified > > in the children nodes of LPC and we are not using a quirk function > > (we are just checking that we have the indirect io assigned and > > that we are on a ISA bus). Now probably there is a nit in my > > code sketch where of_isa_indirect_io should be probably an > architecture > > specific function... > > But the point is that it would then return an incorrect address, > which in the worst case could be the same as another I/O space > if that happens to be at CPU address zero. If we do not touch __of_address_to_resource after taddr is returned by of_translate_address we will check for (flags & IORESOURCE_IO), then we call pci_address_to_pio to retrieve the unique token (remember that LPC driver will register the LPC io range to pci io_range_list). I do not think that we can have any conflict with any other I/O space as pci_register_io_range will guarantee that the LPC range does not overlap with any other I/O range... > > > > I think all we need from this function is to return '1' if > > > we hit an ISA I/O window, and that should happen for the two > > > interesting cases, either no 'ranges' at all, or no translation > > > for the range in question, so that __of_translate_address can > > > return OF_BAD_ADDR, and we can enter the special case > > > handling in the caller, that handles it like > > > > > > > I don't think this is very right as you may fail for different > > reasons other than a missing range property, e.g: > > http://lxr.free-electrons.com/source/drivers/of/address.c#L575 > > > > And even if the only failure case was a missing range if in the > > future __of_translate_address had to be reworked we would again > > make a wrong assumption...you get my point? > > The newly introduced function would clearly have to make > some sanity checks. The idea is that treat the case of > not being able to translate a bus specific I/O address > into a CPU address literally and fall back to another method > of translating that address. > > This matches my mental model of how we find the resource: > > - start with the bus address > - try to translate that into a CPU address > - if we arrive at a CPU physical address for IORESOURCE_MEM, use that > - if we arrive at a CPU physical address for IORESOURCE_IO, translate > that into a Linux IORESOURCE_IO token > - if there is no valid CPU physical address, try to translate > the address into an IORESOURCE_IO using the ISA accessor > - if that fails too, give up. > > If you try to fake a CPU physical address inbetween, it just > gets more confusing. > > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/22/2016 11:20 PM, Gabriele Paoloni wrote: > >> -----Original Message----- >> From: Arnd Bergmann [mailto:arnd@arndb.de] >> Sent: 22 September 2016 15:59 >> To: Gabriele Paoloni >> Cc: zhichang; linux-arm-kernel@lists.infradead.org; >> devicetree@vger.kernel.org; lorenzo.pieralisi@arm.com; minyard@acm.org; >> linux-pci@vger.kernel.org; gregkh@linuxfoundation.org; John Garry; >> will.deacon@arm.com; linux-kernel@vger.kernel.org; Yuanzhichang; >> Linuxarm; xuwei (O); linux-serial@vger.kernel.org; >> benh@kernel.crashing.org; zourongrong@gmail.com; liviu.dudau@arm.com; >> kantyzc@163.com >> Subject: Re: [PATCH V3 2/4] ARM64 LPC: LPC driver implementation on >> Hip06 >> >> On Thursday, September 22, 2016 2:47:14 PM CEST Gabriele Paoloni wrote: >>>>> static int of_empty_ranges_quirk(struct device_node *np) >>>>> { >>>>> if (IS_ENABLED(CONFIG_PPC)) { >>>>> @@ -503,7 +512,7 @@ static int of_translate_one(struct >> device_node >>>> *parent, struct of_bus *bus, >>>>> * This code is only enabled on powerpc. --gcl >>>>> */ >>>>> ranges = of_get_property(parent, rprop, &rlen); >>>>> - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { >>>>> + if (ranges == NULL && !of_empty_ranges_quirk(parent) && >>>> !of_isa_indirect_io(parent)) { >>>>> pr_debug("OF: no ranges; cannot translate\n"); >>>>> return 1; >>>>> } >>>> I don't see what effect that would have. What do you want to >>>> achieve with this? >>> If I read the code correctly adding the function above would end >>> up in a 1:1 mapping: >>> http://lxr.free-electrons.com/source/drivers/of/address.c#L513 >>> >>> so taddr will be assigned with the cpu address space specified >>> in the children nodes of LPC and we are not using a quirk function >>> (we are just checking that we have the indirect io assigned and >>> that we are on a ISA bus). Now probably there is a nit in my >>> code sketch where of_isa_indirect_io should be probably an >> architecture >>> specific function... >> But the point is that it would then return an incorrect address, >> which in the worst case could be the same as another I/O space >> if that happens to be at CPU address zero. > If we do not touch __of_address_to_resource after taddr is returned > by of_translate_address we will check for (flags & IORESOURCE_IO), > then we call pci_address_to_pio to retrieve the unique token (remember > that LPC driver will register the LPC io range to pci io_range_list). > > I do not think that we can have any conflict with any other I/O space > as pci_register_io_range will guarantee that the LPC range does not > overlap with any other I/O range... If we don't bypass the calling of pci_address_to_pio after of_translate_address, there should no conflict between LPC logical IO range and other logical IO ranges of other devices. I guess Arnd want to skip all the translation for our LPC IO address. But if we do it like that, it seems we can't avoid the possible conflict with the logical IO ranges of PCI host bridges without any changes on the pci_register_io_range and pci_address_to_pio. Because two completely separate I/O spaces are created without synchronization. Best, Zhichang >>>> I think all we need from this function is to return '1' if >>>> we hit an ISA I/O window, and that should happen for the two >>>> interesting cases, either no 'ranges' at all, or no translation >>>> for the range in question, so that __of_translate_address can >>>> return OF_BAD_ADDR, and we can enter the special case >>>> handling in the caller, that handles it like >>>> >>> I don't think this is very right as you may fail for different >>> reasons other than a missing range property, e.g: >>> http://lxr.free-electrons.com/source/drivers/of/address.c#L575 >>> >>> And even if the only failure case was a missing range if in the >>> future __of_translate_address had to be reworked we would again >>> make a wrong assumption...you get my point? >> The newly introduced function would clearly have to make >> some sanity checks. The idea is that treat the case of >> not being able to translate a bus specific I/O address >> into a CPU address literally and fall back to another method >> of translating that address. >> >> This matches my mental model of how we find the resource: >> >> - start with the bus address >> - try to translate that into a CPU address >> - if we arrive at a CPU physical address for IORESOURCE_MEM, use that >> - if we arrive at a CPU physical address for IORESOURCE_IO, translate >> that into a Linux IORESOURCE_IO token >> - if there is no valid CPU physical address, try to translate >> the address into an IORESOURCE_IO using the ISA accessor >> - if that fails too, give up. >> >> If you try to fake a CPU physical address inbetween, it just >> gets more confusing. >> >> Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/22/2016 08:14 PM, Arnd Bergmann wrote: > On Thursday, September 22, 2016 11:55:45 AM CEST Gabriele Paoloni wrote: >>>> I think extending of_empty_ranges_quirk() may be a reasonable >>> solution. >>>> What do you think Arnd? >>> I don't really like that idea, that quirk is meant to work around >>> broken DTs, but we can just make the DT valid and implement the >>> code properly. >> Ok I understand your point where it is not right to use of_empty_ranges_quirk() >> As a quirk is used to work around broken HW or broken FW (as in this case) >> rather than to fix code >> >> What about the following? I think adding the check you suggested next to >> of_empty_ranges_quirk() is adding the case we need in the right point (thus >> avoiding any duplication) >> >> --- a/drivers/of/address.c >> +++ b/drivers/of/address.c >> @@ -457,6 +457,15 @@ static struct of_bus *of_match_bus(struct device_node *np) >> return NULL; >> } >> >> +static inline int of_isa_indirect_io(struct device_node *np) >> +{ >> + /* >> + * check if the current node is an isa bus and if indirectio operation >> + * are registered >> + */ >> + return (of_bus_isa_match(np) && arm64_extio_ops); >> +} >> + >> static int of_empty_ranges_quirk(struct device_node *np) >> { >> if (IS_ENABLED(CONFIG_PPC)) { >> @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus, >> * This code is only enabled on powerpc. --gcl >> */ >> ranges = of_get_property(parent, rprop, &rlen); >> - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { >> + if (ranges == NULL && !of_empty_ranges_quirk(parent) && !of_isa_indirect_io(parent)) { >> pr_debug("OF: no ranges; cannot translate\n"); >> return 1; >> } > I don't see what effect that would have. What do you want to > achieve with this? > > I think all we need from this function is to return '1' if > we hit an ISA I/O window, and that should happen for the two > interesting cases, either no 'ranges' at all, or no translation > for the range in question, so that __of_translate_address can > return OF_BAD_ADDR, and we can enter the special case > handling in the caller, that handles it like > > diff --git a/drivers/of/address.c b/drivers/of/address.c > index 02b2903fe9d2..a18d96843fae 100644 > --- a/drivers/of/address.c > +++ b/drivers/of/address.c > @@ -685,17 +685,24 @@ static int __of_address_to_resource(struct device_node *dev, > if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0) > return -EINVAL; > taddr = of_translate_address(dev, addrp); > - if (taddr == OF_BAD_ADDR) > - return -EINVAL; > memset(r, 0, sizeof(struct resource)); > + > if (flags & IORESOURCE_IO) { > unsigned long port; > - port = pci_address_to_pio(taddr); > + > + if (taddr == OF_BAD_ADDR) > + port = arch_of_address_to_pio(dev, addrp) > + else > + port = pci_address_to_pio(taddr); > + > if (port == (unsigned long)-1) > return -EINVAL; > r->start = port; > r->end = port + size - 1; > } else { > + if (taddr == OF_BAD_ADDR) > + return -EINVAL; > + > r->start = taddr; > r->end = taddr + size - 1; > } > For this patch sketch, I have a question. Do we call pci_address_to_pio in arch_of_address_to_pio to get the corresponding logical IO port for LPC?? If we don't, it seems the LPC specific IO address will conflict with PCI host bridges' logical IO. Supposed our LPC populated the IO range from 0x100 to 0x3FF( this is normal for ISA similar devices), after arch_of_address_to_pio(), the r->start will be set as 0x100, r->end will be set as 0x3FF. And if there is one PCI host bridge who request a IO window size over 0x400 at the same time, the corresponding r->start and r->end will be set as 0x0, 0x3FF after of_address_to_resource for this host bridge. Then the IO conflict happens. cheers, Zhichang > > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -457,6 +457,15 @@ static struct of_bus *of_match_bus(struct device_node *np) return NULL; } +static inline int of_isa_indirect_io(struct device_node *np) +{ + /* + * check if the current node is an isa bus and if indirectio operation + * are registered + */ + return (of_bus_isa_match(np) && arm64_extio_ops); +} + static int of_empty_ranges_quirk(struct device_node *np) { if (IS_ENABLED(CONFIG_PPC)) { @@ -503,7 +512,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus, * This code is only enabled on powerpc. --gcl */ ranges = of_get_property(parent, rprop, &rlen); - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { + if (ranges == NULL && !of_empty_ranges_quirk(parent) && !of_isa_indirect_io(parent)) { pr_debug("OF: no ranges; cannot translate\n"); return 1; }